From a0da41f480e0b44af337af31b6af471cb5353f17 Mon Sep 17 00:00:00 2001 From: Ammar Date: Mon, 6 Apr 2026 08:48:02 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=A4=96=20refactor:=20clean=20up=20workspa?= =?UTF-8?q?ce=20status=20handoff=20rendering?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the mirrored timeout from the sidebar handoff slot, clear the transient collapse state when another status row hides the slot, and fold the related spec back down so the follow-up diff stays net negative. --- _Generated with `mux` • Model: `openai:gpt-5.4` • Thinking: `xhigh` • Cost: `$19.05`_ --- .../WorkspaceStatusIndicator.test.tsx | 191 +++++++++--------- .../WorkspaceStatusIndicator.tsx | 46 ++--- 2 files changed, 115 insertions(+), 122 deletions(-) diff --git a/src/browser/components/WorkspaceStatusIndicator/WorkspaceStatusIndicator.test.tsx b/src/browser/components/WorkspaceStatusIndicator/WorkspaceStatusIndicator.test.tsx index 770f3f30e9..16d19d636a 100644 --- a/src/browser/components/WorkspaceStatusIndicator/WorkspaceStatusIndicator.test.tsx +++ b/src/browser/components/WorkspaceStatusIndicator/WorkspaceStatusIndicator.test.tsx @@ -1,18 +1,21 @@ import "../../../../tests/ui/dom"; import { afterEach, beforeEach, describe, expect, mock, spyOn, test } from "bun:test"; -import { cleanup, render } from "@testing-library/react"; +import { cleanup, fireEvent, render } from "@testing-library/react"; import { installDom } from "../../../../tests/ui/dom"; import * as WorkspaceStoreModule from "@/browser/stores/WorkspaceStore"; - import { formatModelDisplayName } from "@/common/utils/ai/modelDisplay"; import { getModelName } from "@/common/utils/ai/models"; import { WorkspaceStatusIndicator } from "./WorkspaceStatusIndicator"; -function mockSidebarState( +const FALLBACK_MODEL = "anthropic:claude-sonnet-4-5"; +const PENDING_MODEL = "openai:gpt-4o-mini"; +const PENDING_DISPLAY_NAME = formatModelDisplayName(getModelName(PENDING_MODEL)); + +function createSidebarState( overrides: Partial = {} -): void { - spyOn(WorkspaceStoreModule, "useWorkspaceSidebarState").mockImplementation(() => ({ +): WorkspaceStoreModule.WorkspaceSidebarState { + return { canInterrupt: false, isStarting: false, awaitingUserQuestion: false, @@ -26,7 +29,30 @@ function mockSidebarState( terminalActiveCount: 0, terminalSessionCount: 0, ...overrides, - })); + }; +} + +function renderIndicator( + overrides: Partial = {}, + workspaceId = "workspace" +) { + const state = createSidebarState(overrides); + spyOn(WorkspaceStoreModule, "useWorkspaceSidebarState").mockImplementation(() => state); + const view = render( + + ); + return { + state, + view, + rerender(nextWorkspaceId = workspaceId) { + view.rerender( + + ); + }, + phaseSlot: () => view.container.querySelector("[data-phase-slot]"), + phaseIcon: () => view.container.querySelector("[data-phase-slot] svg"), + modelDisplay: () => view.container.querySelector("[data-model-display]"), + }; } describe("WorkspaceStatusIndicator", () => { @@ -43,107 +69,76 @@ describe("WorkspaceStatusIndicator", () => { mock.restore(); }); - test("keeps unfinished todo status static once the stream is idle", () => { - mockSidebarState({ - agentStatus: { emoji: "🔄", message: "Run checks" }, + for (const [name, overrides, spins] of [ + [ + "keeps unfinished todo status static once the stream is idle", + { agentStatus: { emoji: "🔄", message: "Run checks" } }, + false, + ], + [ + "keeps refresh-style status animated while a stream is still active", + { canInterrupt: true, agentStatus: { emoji: "🔄", message: "Run checks" } }, + true, + ], + ] satisfies Array<[string, Partial, boolean]>) { + test(name, () => { + const { view } = renderIndicator(overrides, name); + const className = view.container.querySelector("svg")?.getAttribute("class") ?? ""; + expect(view.container.querySelector("svg")).toBeTruthy(); + expect(className.includes("animate-spin")).toBe(spins); }); + } - const view = render( - + test("keeps the model label anchored when starting hands off to streaming", () => { + const indicator = renderIndicator( + { isStarting: true, pendingStreamModel: PENDING_MODEL }, + "workspace-phase-shift-starting" ); - - const icon = view.container.querySelector("svg"); - expect(icon).toBeTruthy(); - expect(icon?.getAttribute("class") ?? "").not.toContain("animate-spin"); - }); - - test("keeps refresh-style status animated while a stream is still active", () => { - mockSidebarState({ + expect(indicator.phaseSlot()?.getAttribute("class") ?? "").toContain("w-3"); + expect(indicator.phaseSlot()?.getAttribute("class") ?? "").toContain("mr-1.5"); + expect(indicator.phaseIcon()?.getAttribute("class") ?? "").toContain("animate-spin"); + expect(indicator.modelDisplay()?.textContent ?? "").toContain(PENDING_DISPLAY_NAME); + expect(indicator.view.container.textContent?.toLowerCase()).toContain("starting"); + + Object.assign(indicator.state, { + isStarting: false, canInterrupt: true, - agentStatus: { emoji: "🔄", message: "Run checks" }, + currentModel: PENDING_MODEL, + pendingStreamModel: null, }); + indicator.rerender("workspace-phase-shift-streaming"); + + expect(indicator.phaseSlot()?.getAttribute("class") ?? "").toContain("w-0"); + expect(indicator.phaseSlot()?.getAttribute("class") ?? "").toContain("mr-0"); + expect(indicator.phaseIcon()?.getAttribute("class") ?? "").not.toContain("animate-spin"); + fireEvent.transitionEnd(indicator.phaseSlot()!, { propertyName: "width" }); + expect(indicator.phaseSlot()).toBeNull(); + expect(indicator.modelDisplay()?.textContent ?? "").toContain(PENDING_DISPLAY_NAME); + expect(indicator.view.container.textContent?.toLowerCase()).toContain("streaming"); + }); - const view = render( - + test("does not leak the collapsed handoff slot after agent status hides it", () => { + const indicator = renderIndicator( + { isStarting: true, pendingStreamModel: PENDING_MODEL }, + "workspace-status-handoff-starting" ); + expect(indicator.phaseSlot()?.getAttribute("class") ?? "").toContain("w-3"); - const icon = view.container.querySelector("svg"); - expect(icon).toBeTruthy(); - expect(icon?.getAttribute("class") ?? "").toContain("animate-spin"); - }); - - test("keeps the steady streaming layout free of the transient handoff slot", () => { - mockSidebarState({ + Object.assign(indicator.state, { + isStarting: false, canInterrupt: true, - currentModel: "openai:gpt-4o-mini", + currentModel: PENDING_MODEL, + pendingStreamModel: null, + agentStatus: { emoji: "🔄", message: "Run checks" }, }); - - const view = render( - - ); - - expect(view.container.querySelector("[data-phase-slot]")).toBeNull(); - expect(view.container.textContent?.toLowerCase()).toContain("streaming"); - }); - - test("keeps the model label anchored when starting hands off to streaming", () => { - const pendingModel = "openai:gpt-4o-mini"; - const fallbackModel = "anthropic:claude-sonnet-4-5"; - const pendingDisplayName = formatModelDisplayName(getModelName(pendingModel)); - const fallbackDisplayName = formatModelDisplayName(getModelName(fallbackModel)); - const state: WorkspaceStoreModule.WorkspaceSidebarState = { - canInterrupt: false, - isStarting: true, - awaitingUserQuestion: false, - lastAbortReason: null, - currentModel: null, - pendingStreamModel: pendingModel, - recencyTimestamp: null, - loadedSkills: [], - skillLoadErrors: [], - agentStatus: undefined, - terminalActiveCount: 0, - terminalSessionCount: 0, - }; - spyOn(WorkspaceStoreModule, "useWorkspaceSidebarState").mockImplementation(() => state); - - const view = render( - - ); - - const getPhaseSlot = () => view.container.querySelector("[data-phase-slot]"); - const getPhaseIcon = () => getPhaseSlot()?.querySelector("svg"); - const getModelDisplay = () => view.container.querySelector("[data-model-display]"); - - expect(getPhaseSlot()?.getAttribute("class") ?? "").toContain("w-3"); - expect(getPhaseSlot()?.getAttribute("class") ?? "").toContain("mr-1.5"); - expect(getPhaseIcon()?.getAttribute("class") ?? "").toContain("animate-spin"); - expect(getModelDisplay()?.textContent ?? "").toContain(pendingDisplayName); - expect(getModelDisplay()?.textContent ?? "").not.toContain(fallbackDisplayName); - expect(view.container.textContent?.toLowerCase()).toContain("starting"); - - state.isStarting = false; - state.canInterrupt = true; - state.currentModel = pendingModel; - state.pendingStreamModel = null; - view.rerender( - - ); - - expect(getPhaseSlot()?.getAttribute("class") ?? "").toContain("w-0"); - expect(getPhaseSlot()?.getAttribute("class") ?? "").toContain("mr-0"); - expect(getPhaseIcon()?.getAttribute("class") ?? "").not.toContain("animate-spin"); - expect(getModelDisplay()?.textContent ?? "").toContain(pendingDisplayName); - expect(getModelDisplay()?.textContent ?? "").not.toContain(fallbackDisplayName); - expect(view.container.textContent?.toLowerCase()).toContain("streaming"); + indicator.rerender("workspace-status-handoff-status"); + expect(indicator.phaseSlot()).toBeNull(); + expect(indicator.view.container.textContent ?? "").toContain("Run checks"); + + indicator.state.agentStatus = undefined; + indicator.rerender("workspace-status-handoff-streaming"); + expect(indicator.phaseSlot()).toBeNull(); + expect(indicator.modelDisplay()?.textContent ?? "").toContain(PENDING_DISPLAY_NAME); + expect(indicator.view.container.textContent?.toLowerCase()).toContain("streaming"); }); }); diff --git a/src/browser/components/WorkspaceStatusIndicator/WorkspaceStatusIndicator.tsx b/src/browser/components/WorkspaceStatusIndicator/WorkspaceStatusIndicator.tsx index 5879644e23..80cddacc0f 100644 --- a/src/browser/components/WorkspaceStatusIndicator/WorkspaceStatusIndicator.tsx +++ b/src/browser/components/WorkspaceStatusIndicator/WorkspaceStatusIndicator.tsx @@ -29,23 +29,37 @@ export const WorkspaceStatusIndicator = memo<{ const previousPhaseRef = useRef(phase); const [isCollapsingPhaseSlot, setIsCollapsingPhaseSlot] = useState(false); + const phaseSlotBlocked = awaitingUserQuestion || Boolean(agentStatus); const shouldCollapsePhaseSlot = isCollapsingPhaseSlot || (previousPhaseRef.current === "starting" && phase === "streaming"); + const showPhaseSlot = !phaseSlotBlocked && (phase === "starting" || shouldCollapsePhaseSlot); useEffect(() => { const previousPhase = previousPhaseRef.current; previousPhaseRef.current = phase; + if (phaseSlotBlocked) { + setIsCollapsingPhaseSlot(false); + return; + } + if (previousPhase === "starting" && phase === "streaming") { setIsCollapsingPhaseSlot(true); - const timeoutId = window.setTimeout(() => { - setIsCollapsingPhaseSlot(false); - }, 150); - return () => window.clearTimeout(timeoutId); + return; + } + + if (phase !== "streaming") { + setIsCollapsingPhaseSlot(false); } + }, [phase, phaseSlotBlocked]); - setIsCollapsingPhaseSlot(false); - }, [phase]); + // Let the CSS transition decide when the handoff slot can disappear so the JS logic + // does not need a mirrored timeout that can drift from the rendered duration. + const handlePhaseSlotTransitionEnd = () => { + if (phase === "streaming" && isCollapsingPhaseSlot) { + setIsCollapsingPhaseSlot(false); + } + }; // Show prompt when ask_user_question is pending - make it prominent if (awaitingUserQuestion) { @@ -109,28 +123,11 @@ export const WorkspaceStatusIndicator = memo<{ : (currentModel ?? pendingStreamModel ?? fallbackModel); const suffix = phase === "starting" ? "- starting..." : "- streaming..."; - if (phase === "streaming" && !shouldCollapsePhaseSlot) { - return ( -
- {modelToShow ? ( - <> - - - - {suffix} - - ) : ( - Assistant - streaming... - )} -
- ); - } - return (
{/* Keep the old steady-state layout, but hold the spinner slot just long enough to animate the start -> stream handoff instead of flashing the label left. */} - {(phase === "starting" || shouldCollapsePhaseSlot) && ( + {showPhaseSlot && (