From caeda15cf67436c8cbf42c7034cf089c8a19f4d4 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 31 May 2026 15:12:27 -0500 Subject: [PATCH 1/7] =?UTF-8?q?=F0=9F=A4=96=20feat:=20immersive=20review?= =?UTF-8?q?=20assisted-mode=20badge=20+=20agent=20status=20bar?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surface assisted-filter mode and chat status inside full-screen immersive review. --- .../ImmersiveReviewAgentStatusBar.test.tsx | 211 ++++++++++++++++++ .../ImmersiveReviewAgentStatusBar.tsx | 182 +++++++++++++++ .../ImmersiveReviewView.stories.tsx | 38 ++++ .../CodeReview/ImmersiveReviewView.test.tsx | 19 ++ .../CodeReview/ImmersiveReviewView.tsx | 38 ++++ .../RightSidebar/CodeReview/ReviewPanel.tsx | 3 + src/common/constants/storage.ts | 12 + 7 files changed, 503 insertions(+) create mode 100644 src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.test.tsx create mode 100644 src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx diff --git a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.test.tsx b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.test.tsx new file mode 100644 index 0000000000..cb3db916bb --- /dev/null +++ b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.test.tsx @@ -0,0 +1,211 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { cleanup, fireEvent, render, type RenderResult } from "@testing-library/react"; +import { GlobalWindow } from "happy-dom"; + +import { readPersistedState } from "@/browser/hooks/usePersistedState"; +import { + useWorkspaceStoreRaw as getWorkspaceStoreRaw, + type WorkspaceSidebarState, + type WorkspaceState, +} from "@/browser/stores/WorkspaceStore"; +import { getImmersiveReviewAgentBarExpandedKey } from "@/common/constants/storage"; +import type { TodoItem } from "@/common/types/tools"; +import { ImmersiveReviewAgentStatusBar } from "./ImmersiveReviewAgentStatusBar"; + +interface SeedInput { + todos: TodoItem[]; + canInterrupt?: boolean; + isStarting?: boolean; + awaitingUserQuestion?: boolean; +} + +interface SeedCache { + state: WorkspaceState; + sidebar: WorkspaceSidebarState; +} + +// Cache the built snapshots per workspace so getWorkspaceState/getWorkspaceSidebarState +// return referentially-stable objects (useSyncExternalStore would otherwise loop). +const seeds = new Map(); +const subscribers = new Map void>>(); + +function getSubscribers(workspaceId: string): Set<() => void> { + let set = subscribers.get(workspaceId); + if (!set) { + set = new Set(); + subscribers.set(workspaceId, set); + } + return set; +} + +function buildState(workspaceId: string, input: SeedInput): WorkspaceState { + return { + name: workspaceId, + messages: [], + queuedMessage: null, + canInterrupt: input.canInterrupt ?? false, + isCompacting: false, + isStreamStarting: input.isStarting ?? false, + awaitingUserQuestion: input.awaitingUserQuestion ?? false, + loading: false, + isHydratingTranscript: false, + hasOlderHistory: false, + loadingOlderHistory: false, + muxMessages: [], + currentModel: null, + currentThinkingLevel: null, + recencyTimestamp: null, + todos: input.todos, + loadedSkills: [], + skillLoadErrors: [], + agentStatus: undefined, + lastAbortReason: null, + pendingStreamStartTime: null, + pendingStreamModel: null, + runtimeStatus: null, + autoRetryStatus: null, + }; +} + +function buildSidebar(input: SeedInput): WorkspaceSidebarState { + return { + canInterrupt: input.canInterrupt ?? false, + isStarting: input.isStarting ?? false, + awaitingUserQuestion: input.awaitingUserQuestion ?? false, + lastAbortReason: null, + currentModel: null, + pendingStreamModel: null, + recencyTimestamp: null, + loadedSkills: [], + skillLoadErrors: [], + agentStatus: undefined, + terminalActiveCount: 0, + terminalSessionCount: 0, + goal: null, + }; +} + +function seed(workspaceId: string, input: SeedInput): void { + seeds.set(workspaceId, { + state: buildState(workspaceId, input), + sidebar: buildSidebar(input), + }); +} + +const store = getWorkspaceStoreRaw(); +const original = { + hasRegisteredWorkspace: store.hasRegisteredWorkspace.bind(store), + subscribeKey: store.subscribeKey.bind(store), + getWorkspaceState: store.getWorkspaceState.bind(store), + getWorkspaceSidebarState: store.getWorkspaceSidebarState.bind(store), +}; + +function renderBar(workspaceId: string): RenderResult { + return render(); +} + +describe("ImmersiveReviewAgentStatusBar", () => { + let originalWindow: typeof globalThis.window; + let originalDocument: typeof globalThis.document; + let originalLocalStorage: typeof globalThis.localStorage; + + beforeEach(() => { + originalWindow = globalThis.window; + originalDocument = globalThis.document; + originalLocalStorage = globalThis.localStorage; + + globalThis.window = new GlobalWindow() as unknown as Window & typeof globalThis; + globalThis.document = globalThis.window.document; + globalThis.localStorage = globalThis.window.localStorage; + globalThis.localStorage.clear(); + seeds.clear(); + subscribers.clear(); + + store.hasRegisteredWorkspace = (id: string) => seeds.has(id); + store.subscribeKey = (id: string, cb: () => void) => { + const set = getSubscribers(id); + set.add(cb); + return () => { + set.delete(cb); + }; + }; + store.getWorkspaceState = (id: string) => { + const cache = seeds.get(id); + if (!cache) throw new Error(`Missing seed for ${id}`); + return cache.state; + }; + store.getWorkspaceSidebarState = (id: string) => { + const cache = seeds.get(id); + if (!cache) throw new Error(`Missing seed for ${id}`); + return cache.sidebar; + }; + }); + + afterEach(() => { + cleanup(); + store.hasRegisteredWorkspace = original.hasRegisteredWorkspace; + store.subscribeKey = original.subscribeKey; + store.getWorkspaceState = original.getWorkspaceState; + store.getWorkspaceSidebarState = original.getWorkspaceSidebarState; + globalThis.window = originalWindow; + globalThis.document = originalDocument; + globalThis.localStorage = originalLocalStorage; + seeds.clear(); + subscribers.clear(); + }); + + const todos: TodoItem[] = [ + { content: "Wire up status bar", status: "in_progress" }, + { content: "Add tests", status: "pending" }, + ]; + + test("renders the TODO plan (expanded) when todos exist", () => { + seed("ws-todos", { todos }); + const result = renderBar("ws-todos"); + // Vertical TodoList content is visible by default. + expect(result.getByText("Wire up status bar")).toBeTruthy(); + expect(result.getByText("Add tests")).toBeTruthy(); + // Summary reflects the counts. + expect(result.getByText(/1 in progress/)).toBeTruthy(); + }); + + test("renders nothing when there is no plan and no active stream", () => { + seed("ws-idle", { todos: [] }); + const result = renderBar("ws-idle"); + expect(result.container.firstChild).toBeNull(); + }); + + test("shows a streaming chip even when there is no plan yet", () => { + seed("ws-streaming", { todos: [], canInterrupt: true }); + const result = renderBar("ws-streaming"); + expect(result.getByText("Streaming…")).toBeTruthy(); + // No plan means no TODO summary / expand toggle. + expect(result.queryByText("TODO")).toBeNull(); + }); + + test("shows a starting chip during pre-stream startup", () => { + seed("ws-starting", { todos: [], isStarting: true }); + const result = renderBar("ws-starting"); + expect(result.getByText("Starting…")).toBeTruthy(); + }); + + test("surfaces a prominent prompt when the agent awaits a question", () => { + seed("ws-question", { todos, awaitingUserQuestion: true }); + const result = renderBar("ws-question"); + expect(result.getByText("Mux has a question")).toBeTruthy(); + // The question chip wins over the streaming label. + expect(result.queryByText("Streaming…")).toBeNull(); + }); + + test("collapsing hides the plan and persists the choice", () => { + const workspaceId = "ws-collapse"; + seed(workspaceId, { todos }); + const result = renderBar(workspaceId); + + fireEvent.click(result.getByRole("button", { name: /todo/i })); + expect(result.queryByText("Wire up status bar")).toBeNull(); + expect(readPersistedState(getImmersiveReviewAgentBarExpandedKey(workspaceId), true)).toBe( + false + ); + }); +}); diff --git a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx new file mode 100644 index 0000000000..37cd310638 --- /dev/null +++ b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx @@ -0,0 +1,182 @@ +/** + * ImmersiveReviewAgentStatusBar — pinned to the top of full-screen immersive + * review. While the user reviews code in immersive mode the chat transcript and + * composer-adjacent status (TODO plan, streaming barrier) are hidden behind the + * opaque overlay, so a common workflow — reviewing while waiting on the agent — + * loses all signal about what the agent is doing. + * + * This bar restores that signal without leaving immersive: + * - the agent's TODO plan in a vertical layout (collapsible, persisted), and + * - live streaming status (starting / streaming / awaiting a question). + * + * Design notes: + * - Subscriptions live in this leaf component (not in ImmersiveReviewView) so + * per-token streaming/todo churn doesn't re-render the large diff tree. + * - Flash-free: the streaming chip is gated on the *held* phase from + * useWorkspaceStreamingStatusPhase (150ms), so brief starting<->streaming + * handoffs don't blink. Because TODO plans persist across streams, the bar + * stays mounted between turns and only unmounts once both the held phase + * clears AND there are no todos left to show — no mid-review flicker. + * - Crash-safe: when the workspace isn't registered in the store yet (tests, + * storybook, teardown) the subscriptions fall back to empty/idle instead of + * throwing, so the bar simply renders nothing. + */ + +import React, { useSyncExternalStore } from "react"; +import { ChevronDown, ChevronRight, CircleHelp, List, Loader2 } from "lucide-react"; +import { TodoList } from "@/browser/components/TodoList/TodoList"; +import { + useOptionalWorkspaceSidebarState, + useWorkspaceStoreRaw, +} from "@/browser/stores/WorkspaceStore"; +import { + getWorkspaceStreamingStatusPhase, + useWorkspaceStreamingStatusPhase, +} from "@/browser/hooks/useWorkspaceStreamingStatusPhase"; +import { usePersistedState } from "@/browser/hooks/usePersistedState"; +import { getImmersiveReviewAgentBarExpandedKey } from "@/common/constants/storage"; +import type { TodoItem } from "@/common/types/tools"; + +// Stable empty reference so useSyncExternalStore's snapshot stays referentially +// stable when the workspace isn't registered — returning a fresh [] each call +// would trip the store's "getSnapshot should be cached" infinite-loop guard. +const EMPTY_TODOS: TodoItem[] = []; + +interface ImmersiveReviewAgentStatusBarProps { + workspaceId: string; +} + +export const ImmersiveReviewAgentStatusBar: React.FC = ({ + workspaceId, +}) => { + const [expanded, setExpanded] = usePersistedState( + getImmersiveReviewAgentBarExpandedKey(workspaceId), + true + ); + + const workspaceStore = useWorkspaceStoreRaw(); + const todos = useSyncExternalStore( + (callback) => + workspaceStore.hasRegisteredWorkspace(workspaceId) + ? workspaceStore.subscribeKey(workspaceId, callback) + : () => undefined, + () => + workspaceStore.hasRegisteredWorkspace(workspaceId) + ? workspaceStore.getWorkspaceState(workspaceId).todos + : EMPTY_TODOS + ); + + const sidebarState = useOptionalWorkspaceSidebarState(workspaceId); + const canInterrupt = sidebarState?.canInterrupt ?? false; + const isStarting = sidebarState?.isStarting ?? false; + const awaitingUserQuestion = sidebarState?.awaitingUserQuestion ?? false; + + // Held phase keeps the streaming chip steady across the starting->streaming + // handoff so it doesn't blink out for a frame between adjacent state settles. + const phase = getWorkspaceStreamingStatusPhase({ canInterrupt, isStarting }); + const phaseSource = canInterrupt ? "streaming" : isStarting ? "pre-stream" : null; + const { displayPhase } = useWorkspaceStreamingStatusPhase(phase, phaseSource); + + const hasTodos = todos.length > 0; + const isStreamingStatusVisible = displayPhase !== null || awaitingUserQuestion; + + // Nothing to surface: don't reserve any vertical space in the review viewport. + if (!hasTodos && !isStreamingStatusVisible) { + return null; + } + + const inProgressCount = todos.filter((todo) => todo.status === "in_progress").length; + const pendingCount = todos.filter((todo) => todo.status === "pending").length; + const completedCount = todos.length - inProgressCount - pendingCount; + const summaryParts: string[] = []; + if (inProgressCount > 0) { + summaryParts.push(`${inProgressCount} in progress`); + } + if (pendingCount > 0) { + summaryParts.push(`${pendingCount} pending`); + } + if (summaryParts.length === 0 && hasTodos) { + summaryParts.push(`${completedCount} completed`); + } + + // role=status + aria-live so screen readers announce streaming/question + // transitions while the user is focused on the diff. + const statusChip = (() => { + if (awaitingUserQuestion) { + return ( + + + ); + } + if (displayPhase === "starting") { + return ( + + + ); + } + if (displayPhase === "streaming") { + return ( + + + ); + } + return null; + })(); + + const trailingStatus = ( +
+ {statusChip} +
+ ); + + return ( +
+ {hasTodos ? ( + + ) : ( + // Streaming/question only (no plan yet): static row, nothing to expand. +
{trailingStatus}
+ )} + {hasTodos && expanded && ( +
+ +
+ )} +
+ ); +}; diff --git a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.stories.tsx b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.stories.tsx index 586b44160e..e5904a4890 100644 --- a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.stories.tsx +++ b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.stories.tsx @@ -289,6 +289,10 @@ interface ImmersiveReviewStoryProps { assistedHunkIds?: ReadonlySet; /** Per-hunk agent comments — drives the immersive assisted-review banner. */ assistedCommentByHunkId?: Map; + /** Whether the Assisted worklist filter is active — drives the header badge. */ + assistedOnly?: boolean; + assistedCount?: number; + assistedUnreadCount?: number; } function ImmersiveReviewStory(props: ImmersiveReviewStoryProps) { @@ -340,6 +344,9 @@ function ImmersiveReviewStory(props: ImmersiveReviewStoryProps) { firstSeenMap={props.fixture.firstSeenMap} assistedHunkIds={props.assistedHunkIds} assistedCommentByHunkId={props.assistedCommentByHunkId} + assistedOnly={props.assistedOnly} + assistedCount={props.assistedCount} + assistedUnreadCount={props.assistedUnreadCount} /> ); @@ -547,3 +554,34 @@ export const ImmersiveWithAssistedBanner: Story = { ); }, }; + +/** + * Immersive review with the Assisted worklist filter active. Locks in the + * header "Assisted unread/total" badge (Sparkles + review-accent) that tells + * the user the diff is filtered to agent-flagged hunks — the control bar that + * normally hosts this toggle is hidden behind the immersive overlay. + */ +export const ImmersiveWithAssistedMode: Story = { + render: () => ( + + ), + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + + await waitFor( + () => { + canvas.getByTestId("immersive-review-view"); + canvas.getByTestId("immersive-assisted-mode-badge"); + }, + { timeout: 10_000 } + ); + }, +}; diff --git a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.test.tsx b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.test.tsx index 1b99068951..7f9903e2a5 100644 --- a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.test.tsx +++ b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.test.tsx @@ -213,6 +213,25 @@ describe("ImmersiveReviewView", () => { expect(mockApi.workspace.executeBash).not.toHaveBeenCalled(); }); + test("shows the assisted-mode badge only while the Assisted filter is active", () => { + // Off by default: the badge must not appear when not filtering. + const off = renderImmersiveReview(); + expect(off.queryByTestId("immersive-assisted-mode-badge")).toBeNull(); + cleanup(); + + // On: the header surfaces the badge with the unread/total counts so the + // active filter mode is visible even though the control bar is hidden + // behind the immersive overlay. + const on = renderImmersiveReview({ + assistedOnly: true, + assistedCount: 3, + assistedUnreadCount: 2, + }); + const badge = on.getByTestId("immersive-assisted-mode-badge"); + expect(badge.textContent ?? "").toContain("Assisted"); + expect(badge.textContent ?? "").toContain("2/3"); + }); + test("loads full-file context for an in-budget selected hunk even when another hunk is far away", async () => { const nearHunk = createHunk({ id: "hunk-near", diff --git a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx index 81be637836..deb7a9cf0b 100644 --- a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx +++ b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx @@ -21,6 +21,7 @@ import { import { cn } from "@/common/lib/utils"; import { SelectableDiffRenderer } from "../../Shared/DiffRenderer"; import { ImmersiveMinimap } from "./ImmersiveMinimap"; +import { ImmersiveReviewAgentStatusBar } from "./ImmersiveReviewAgentStatusBar"; import { buildNewLineNumberToIndexMap, buildOldLineNumberToIndexMap, @@ -92,6 +93,18 @@ interface ImmersiveReviewViewProps { * see in the side panel. */ assistedCommentByHunkId?: Map; + /** + * Whether the "Assisted" filter (show only agent-flagged hunks) is active. + * The control bar that hosts this toggle is hidden behind the immersive + * overlay, so we surface a header badge to keep the active filter mode + * visible. Distinct from the per-hunk assisted banner: this means "the + * worklist filter is on", not "this hunk was flagged". + */ + assistedOnly?: boolean; + /** Total agent-flagged hunks (mirrors the control bar's Assisted count). */ + assistedCount?: number; + /** Agent-flagged hunks still unread (mirrors the control bar's count). */ + assistedUnreadCount?: number; } interface InlineComposerRequest { @@ -1885,6 +1898,25 @@ export const ImmersiveReviewView: React.FC = (props) = )} )} + {/* Assisted-mode indicator — the control bar that hosts the Assisted + toggle is hidden behind the immersive overlay, so without this the + user has no way to tell the diff is filtered to agent-flagged hunks. + ml-auto anchors it to the row's trailing edge as a mode indicator. */} + {props.assistedOnly === true && ( +
+
+ )} {allHunks.length > 0 && (
= (props) = )}
+ {/* Agent status bar — keeps the TODO plan + live streaming status visible + while reviewing, since the chat transcript/composer are hidden behind + the immersive overlay. Self-subscribes so its updates don't re-render + the diff tree; renders nothing when there's no plan and no stream. */} + + {/* Assisted-review banner — surfaces the agent's flag + comment when the selected hunk is one the agent pinned for review. We render it between the header and the diff so the focus signal is impossible diff --git a/src/browser/features/RightSidebar/CodeReview/ReviewPanel.tsx b/src/browser/features/RightSidebar/CodeReview/ReviewPanel.tsx index 25fba12254..83f673291e 100644 --- a/src/browser/features/RightSidebar/CodeReview/ReviewPanel.tsx +++ b/src/browser/features/RightSidebar/CodeReview/ReviewPanel.tsx @@ -2595,6 +2595,9 @@ export const ReviewPanel: React.FC = ({ firstSeenMap={firstSeenMap} assistedHunkIds={assistedHunkIdSet} assistedCommentByHunkId={assistedCommentByHunkId} + assistedOnly={filters.assistedOnly} + assistedCount={assistedHunks.length} + assistedUnreadCount={unreadAssistedInDiff} />, root ); diff --git a/src/common/constants/storage.ts b/src/common/constants/storage.ts index c6440a2aaf..0767b0e951 100644 --- a/src/common/constants/storage.ts +++ b/src/common/constants/storage.ts @@ -636,6 +636,18 @@ export function getReviewImmersiveKey(workspaceId: string): string { return `review-immersive:${workspaceId}`; } +/** + * Get the localStorage key for the immersive-review agent status bar expansion + * state (the TODO plan + streaming-status bar pinned to the top of immersive + * review). Persisted per workspace so a user's collapse choice survives + * navigation. Mirrors getPinnedTodoExpandedKey — a transient UI preference, so + * intentionally NOT copied on fork. + * Format: "review-immersive-agentbar-expanded:{workspaceId}" + */ +export function getImmersiveReviewAgentBarExpandedKey(workspaceId: string): string { + return `review-immersive-agentbar-expanded:${workspaceId}`; +} + /** * Get the localStorage key for auto-compaction enabled preference per workspace * Format: "autoCompaction:enabled:{workspaceId}" From aab6fb155225e9da109c9bcade197edc6ea1690d Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 31 May 2026 15:17:09 -0500 Subject: [PATCH 2/7] fix: clean up immersive agent-bar localStorage key on workspace removal --- src/common/constants/storage.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/common/constants/storage.ts b/src/common/constants/storage.ts index 0767b0e951..eced9cc41e 100644 --- a/src/common/constants/storage.ts +++ b/src/common/constants/storage.ts @@ -716,6 +716,7 @@ const EPHEMERAL_WORKSPACE_KEY_FUNCTIONS: Array<(workspaceId: string) => string> getPendingWorkspaceSendErrorKey, getPlanContentKey, // Cache only, no need to preserve on fork getPostCompactionStateKey, // Cache only, no need to preserve on fork + getImmersiveReviewAgentBarExpandedKey, // Transient UI pref; clean up on removal, don't carry on fork ]; /** From 53d297feee3756bc3ba2a3943fc0d155053d15e2 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 31 May 2026 15:38:21 -0500 Subject: [PATCH 3/7] fix: make immersive agent status bar robust to test mock contamination Read todos + streaming flags from a single getWorkspaceState subscription (matching PinnedTodoList) instead of a separate useOptionalWorkspaceSidebarState hook, which other test files globally mock.module and which caused CI-only failures. Rewrite the test to use installDom + spyOn and assert collapse persistence behaviorally via remount. --- .../ImmersiveReviewAgentStatusBar.test.tsx | 151 +++++++----------- .../ImmersiveReviewAgentStatusBar.tsx | 48 ++++-- 2 files changed, 93 insertions(+), 106 deletions(-) diff --git a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.test.tsx b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.test.tsx index cb3db916bb..cbee6a0bf7 100644 --- a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.test.tsx +++ b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.test.tsx @@ -1,14 +1,14 @@ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, mock, spyOn, test } from "bun:test"; import { cleanup, fireEvent, render, type RenderResult } from "@testing-library/react"; -import { GlobalWindow } from "happy-dom"; - -import { readPersistedState } from "@/browser/hooks/usePersistedState"; -import { - useWorkspaceStoreRaw as getWorkspaceStoreRaw, - type WorkspaceSidebarState, - type WorkspaceState, -} from "@/browser/stores/WorkspaceStore"; -import { getImmersiveReviewAgentBarExpandedKey } from "@/common/constants/storage"; + +import { installDom } from "../../../../../tests/ui/dom"; +// Import the module namespace (not the hook directly) so we can spyOn the hook +// per-test. The bar resolves the store via useWorkspaceStoreRaw(), and several +// OTHER test files globally mock.module this store; spying here keeps this file +// hermetic regardless of cross-file load order (grabbing the real singleton at +// module top-level would crash if another file's stub were active first). +import * as WorkspaceStoreModule from "@/browser/stores/WorkspaceStore"; +import type { WorkspaceState, WorkspaceStore } from "@/browser/stores/WorkspaceStore"; import type { TodoItem } from "@/common/types/tools"; import { ImmersiveReviewAgentStatusBar } from "./ImmersiveReviewAgentStatusBar"; @@ -19,14 +19,11 @@ interface SeedInput { awaitingUserQuestion?: boolean; } -interface SeedCache { - state: WorkspaceState; - sidebar: WorkspaceSidebarState; -} - -// Cache the built snapshots per workspace so getWorkspaceState/getWorkspaceSidebarState -// return referentially-stable objects (useSyncExternalStore would otherwise loop). -const seeds = new Map(); +// Cache the built state per workspace so getWorkspaceState returns a +// referentially-stable object (useSyncExternalStore would otherwise loop). +// The bar reads todos + streaming flags off a single getWorkspaceState read +// (matching PinnedTodoList), so there's no separate sidebar-state mock. +const seeds = new Map(); const subscribers = new Map void>>(); function getSubscribers(workspaceId: string): Set<() => void> { @@ -67,89 +64,52 @@ function buildState(workspaceId: string, input: SeedInput): WorkspaceState { }; } -function buildSidebar(input: SeedInput): WorkspaceSidebarState { - return { - canInterrupt: input.canInterrupt ?? false, - isStarting: input.isStarting ?? false, - awaitingUserQuestion: input.awaitingUserQuestion ?? false, - lastAbortReason: null, - currentModel: null, - pendingStreamModel: null, - recencyTimestamp: null, - loadedSkills: [], - skillLoadErrors: [], - agentStatus: undefined, - terminalActiveCount: 0, - terminalSessionCount: 0, - goal: null, - }; -} - function seed(workspaceId: string, input: SeedInput): void { - seeds.set(workspaceId, { - state: buildState(workspaceId, input), - sidebar: buildSidebar(input), - }); + seeds.set(workspaceId, buildState(workspaceId, input)); } -const store = getWorkspaceStoreRaw(); -const original = { - hasRegisteredWorkspace: store.hasRegisteredWorkspace.bind(store), - subscribeKey: store.subscribeKey.bind(store), - getWorkspaceState: store.getWorkspaceState.bind(store), - getWorkspaceSidebarState: store.getWorkspaceSidebarState.bind(store), -}; +// Minimal fake exposing only the store methods the bar calls. Cast through +// unknown because the bar uses just this slice of the WorkspaceStore surface. +const fakeStore = { + hasRegisteredWorkspace: (id: string) => seeds.has(id), + subscribeKey: (id: string, cb: () => void) => { + const set = getSubscribers(id); + set.add(cb); + return () => { + set.delete(cb); + }; + }, + getWorkspaceState: (id: string) => { + const state = seeds.get(id); + if (!state) throw new Error(`Missing seed for ${id}`); + return state; + }, +} as unknown as WorkspaceStore; function renderBar(workspaceId: string): RenderResult { return render(); } describe("ImmersiveReviewAgentStatusBar", () => { - let originalWindow: typeof globalThis.window; - let originalDocument: typeof globalThis.document; - let originalLocalStorage: typeof globalThis.localStorage; + // installDom snapshots + fully restores all DOM globals (window, document, + // localStorage, CustomEvent, …), so this file stays hermetic even when other + // test files in the same process leave the globals in a partial state. + let cleanupDom: (() => void) | null = null; beforeEach(() => { - originalWindow = globalThis.window; - originalDocument = globalThis.document; - originalLocalStorage = globalThis.localStorage; - - globalThis.window = new GlobalWindow() as unknown as Window & typeof globalThis; - globalThis.document = globalThis.window.document; - globalThis.localStorage = globalThis.window.localStorage; + cleanupDom = installDom(); globalThis.localStorage.clear(); seeds.clear(); subscribers.clear(); - store.hasRegisteredWorkspace = (id: string) => seeds.has(id); - store.subscribeKey = (id: string, cb: () => void) => { - const set = getSubscribers(id); - set.add(cb); - return () => { - set.delete(cb); - }; - }; - store.getWorkspaceState = (id: string) => { - const cache = seeds.get(id); - if (!cache) throw new Error(`Missing seed for ${id}`); - return cache.state; - }; - store.getWorkspaceSidebarState = (id: string) => { - const cache = seeds.get(id); - if (!cache) throw new Error(`Missing seed for ${id}`); - return cache.sidebar; - }; + spyOn(WorkspaceStoreModule, "useWorkspaceStoreRaw").mockReturnValue(fakeStore); }); afterEach(() => { cleanup(); - store.hasRegisteredWorkspace = original.hasRegisteredWorkspace; - store.subscribeKey = original.subscribeKey; - store.getWorkspaceState = original.getWorkspaceState; - store.getWorkspaceSidebarState = original.getWorkspaceSidebarState; - globalThis.window = originalWindow; - globalThis.document = originalDocument; - globalThis.localStorage = originalLocalStorage; + mock.restore(); + cleanupDom?.(); + cleanupDom = null; seeds.clear(); subscribers.clear(); }); @@ -197,15 +157,26 @@ describe("ImmersiveReviewAgentStatusBar", () => { expect(result.queryByText("Streaming…")).toBeNull(); }); - test("collapsing hides the plan and persists the choice", () => { + test("collapsing hides the plan and the collapsed choice persists across remounts", () => { const workspaceId = "ws-collapse"; seed(workspaceId, { todos }); - const result = renderBar(workspaceId); - - fireEvent.click(result.getByRole("button", { name: /todo/i })); - expect(result.queryByText("Wire up status bar")).toBeNull(); - expect(readPersistedState(getImmersiveReviewAgentBarExpandedKey(workspaceId), true)).toBe( - false - ); + const first = renderBar(workspaceId); + + // Plan is expanded by default; collapsing hides the vertical list. + expect(first.getByText("Wire up status bar")).toBeTruthy(); + fireEvent.click(first.getByRole("button", { name: /todo/i })); + expect(first.queryByText("Wire up status bar")).toBeNull(); + + // Remounting the bar for the same workspace must restore the collapsed + // choice (asserted via the user-visible re-render rather than reading the + // raw localStorage value, which keeps this resilient to the test-runner's + // shared-global quirks). + first.unmount(); + const second = renderBar(workspaceId); + expect(second.queryByText("Wire up status bar")).toBeNull(); + + // Re-expanding brings the plan back, proving the toggle round-trips. + fireEvent.click(second.getByRole("button", { name: /todo/i })); + expect(second.getByText("Wire up status bar")).toBeTruthy(); }); }); diff --git a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx index 37cd310638..6df1c9bfcd 100644 --- a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx +++ b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx @@ -25,10 +25,7 @@ import React, { useSyncExternalStore } from "react"; import { ChevronDown, ChevronRight, CircleHelp, List, Loader2 } from "lucide-react"; import { TodoList } from "@/browser/components/TodoList/TodoList"; -import { - useOptionalWorkspaceSidebarState, - useWorkspaceStoreRaw, -} from "@/browser/stores/WorkspaceStore"; +import { useWorkspaceStoreRaw } from "@/browser/stores/WorkspaceStore"; import { getWorkspaceStreamingStatusPhase, useWorkspaceStreamingStatusPhase, @@ -37,10 +34,24 @@ import { usePersistedState } from "@/browser/hooks/usePersistedState"; import { getImmersiveReviewAgentBarExpandedKey } from "@/common/constants/storage"; import type { TodoItem } from "@/common/types/tools"; -// Stable empty reference so useSyncExternalStore's snapshot stays referentially -// stable when the workspace isn't registered — returning a fresh [] each call -// would trip the store's "getSnapshot should be cached" infinite-loop guard. -const EMPTY_TODOS: TodoItem[] = []; +/** The slice of WorkspaceState this bar reads. */ +interface AgentStatusSnapshot { + todos: TodoItem[]; + canInterrupt: boolean; + isStreamStarting: boolean; + awaitingUserQuestion: boolean; +} + +// Stable idle snapshot returned when the workspace isn't registered yet (tests, +// storybook, teardown). A module-level constant keeps the useSyncExternalStore +// snapshot referentially stable so the store's "getSnapshot should be cached" +// guard doesn't loop, and the bar simply renders nothing. +const IDLE_SNAPSHOT: AgentStatusSnapshot = { + todos: [], + canInterrupt: false, + isStreamStarting: false, + awaitingUserQuestion: false, +}; interface ImmersiveReviewAgentStatusBarProps { workspaceId: string; @@ -54,22 +65,27 @@ export const ImmersiveReviewAgentStatusBar: React.FC( (callback) => workspaceStore.hasRegisteredWorkspace(workspaceId) ? workspaceStore.subscribeKey(workspaceId, callback) : () => undefined, () => workspaceStore.hasRegisteredWorkspace(workspaceId) - ? workspaceStore.getWorkspaceState(workspaceId).todos - : EMPTY_TODOS + ? workspaceStore.getWorkspaceState(workspaceId) + : IDLE_SNAPSHOT ); - - const sidebarState = useOptionalWorkspaceSidebarState(workspaceId); - const canInterrupt = sidebarState?.canInterrupt ?? false; - const isStarting = sidebarState?.isStarting ?? false; - const awaitingUserQuestion = sidebarState?.awaitingUserQuestion ?? false; + const todos = snapshot.todos; + const canInterrupt = snapshot.canInterrupt; + // Sidebar derives `isStarting` directly from `isStreamStarting`. + const isStarting = snapshot.isStreamStarting; + const awaitingUserQuestion = snapshot.awaitingUserQuestion; // Held phase keeps the streaming chip steady across the starting->streaming // handoff so it doesn't blink out for a frame between adjacent state settles. From df2934046b852a29bda59bc3f503c0599c808a0c Mon Sep 17 00:00:00 2001 From: Ammar Date: Mon, 1 Jun 2026 12:24:59 -0500 Subject: [PATCH 4/7] feat: scope assisted banner to diff column + add agent status bar story - Move the immersive assisted-review comment banner inside the diff column so it spans only the diff width, not across the minimap + notes sidebar. - Add ImmersiveWithAgentStatusBar story that seeds the WorkspaceStore so the top TODO + streaming status bar renders with real state (flash-free). --- .../ImmersiveReviewView.stories.tsx | 133 +++++++++++++ .../CodeReview/ImmersiveReviewView.tsx | 180 +++++++++--------- 2 files changed, 227 insertions(+), 86 deletions(-) diff --git a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.stories.tsx b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.stories.tsx index e5904a4890..95a86df38d 100644 --- a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.stories.tsx +++ b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.stories.tsx @@ -9,6 +9,9 @@ import { ExperimentsProvider } from "@/browser/contexts/ExperimentsContext"; import { ThemeProvider } from "@/browser/contexts/ThemeContext"; import { createMockORPCClient } from "@/browser/stories/mocks/orpc"; import { createReview } from "@/browser/stories/helpers/reviews"; +import { useWorkspaceStoreRaw } from "@/browser/stores/WorkspaceStore"; +import { DEFAULT_RUNTIME_CONFIG } from "@/common/constants/workspace"; +import type { TodoItem } from "@/common/types/tools"; import type { DiffHunk, Review } from "@/common/types/review"; import { extractAllHunks, parseDiff } from "@/common/utils/git/diffParser"; import { @@ -265,6 +268,84 @@ function createImmersiveStoryClient(): APIClient { }); } +interface AgentStatusSeed { + /** TODO plan the agent has written (drives the vertical TODO list + summary). */ + todos: TodoItem[]; + /** + * When true, leaves the seeded stream open so the bar shows the live + * "Streaming…" chip alongside the plan. When false the stream is left + * un-started, so only the persisted (incomplete) plan shows. + */ + streaming?: boolean; +} + +/** + * Register a workspace in the singleton WorkspaceStore and push a `todo_write` + * tool call through its aggregator so the immersive view's + * ImmersiveReviewAgentStatusBar has real plan + streaming state to render. + * Seeds exactly once per workspace (addWorkspace is idempotent and the stream + * events are replayed only when the aggregator is freshly created), and runs + * during render before the child subscribes so the bar paints populated on the + * first frame (flash-free for Chromatic). + */ +function seedAgentStatus( + workspaceStore: ReturnType, + workspaceId: string, + seed: AgentStatusSeed +): void { + const alreadyRegistered = workspaceStore.hasRegisteredWorkspace(workspaceId); + workspaceStore.addWorkspace({ + id: workspaceId, + name: workspaceId, + projectName: "story-project", + projectPath: "/story/project", + namedWorkspacePath: "/story/workspace", + createdAt: new Date(0).toISOString(), + runtimeConfig: DEFAULT_RUNTIME_CONFIG, + }); + if (alreadyRegistered) { + return; + } + + const aggregator = workspaceStore.getAggregator(workspaceId); + if (!aggregator) { + return; + } + + const messageId = `${workspaceId}-stream`; + aggregator.handleStreamStart({ + type: "stream-start", + workspaceId, + messageId, + historySequence: 1, + model: "anthropic:claude-sonnet-4", + startTime: 0, + }); + aggregator.handleToolCallStart({ + type: "tool-call-start", + workspaceId, + messageId, + toolCallId: `${workspaceId}-todo`, + toolName: "todo_write", + args: { todos: seed.todos }, + tokens: 10, + timestamp: 1, + }); + aggregator.handleToolCallEnd({ + type: "tool-call-end", + workspaceId, + messageId, + toolCallId: `${workspaceId}-todo`, + toolName: "todo_write", + result: { success: true }, + timestamp: 2, + }); + if (seed.streaming !== true) { + // Collapse the active stream so the chip clears; incomplete todos persist. + aggregator.clearActiveStreams(); + } +} + const ImmersiveStoryShell: FC<{ client: APIClient; children: ReactNode }> = ({ client, children, @@ -293,6 +374,11 @@ interface ImmersiveReviewStoryProps { assistedOnly?: boolean; assistedCount?: number; assistedUnreadCount?: number; + /** + * Seeds the singleton WorkspaceStore so the immersive view's top status bar + * (ImmersiveReviewAgentStatusBar) renders a real TODO plan + streaming chip. + */ + agentStatusSeed?: AgentStatusSeed; } function ImmersiveReviewStory(props: ImmersiveReviewStoryProps) { @@ -304,6 +390,13 @@ function ImmersiveReviewStory(props: ImmersiveReviewStoryProps) { () => new Set(props.initialReadHunkIds ?? []) ); + // Seed agent status synchronously during render (before the child subscribes) + // so the top status bar paints with its plan/stream on the first frame. + const workspaceStore = useWorkspaceStoreRaw(); + if (props.agentStatusSeed) { + seedAgentStatus(workspaceStore, props.workspaceId, props.agentStatusSeed); + } + return ( ( + + ), + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + + await waitFor( + () => { + canvas.getByTestId("immersive-review-view"); + // The collapsible TODO bar + its vertical plan content render. + canvas.getByTestId("immersive-agent-status-bar"); + canvas.getByText("Add the top status bar (TODO + streaming)"); + // Live streaming chip is visible alongside the plan. + canvas.getByText("Streaming…"); + }, + { timeout: 10_000 } + ); + }, +}; diff --git a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx index deb7a9cf0b..ffdca2d3ef 100644 --- a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx +++ b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx @@ -1963,99 +1963,107 @@ export const ImmersiveReviewView: React.FC = (props) = the diff tree; renders nothing when there's no plan and no stream. */} - {/* Assisted-review banner — surfaces the agent's flag + comment when - the selected hunk is one the agent pinned for review. We render it - between the header and the diff so the focus signal is impossible - to miss after entering immersive mode, where the side-panel cues - aren't visible. */} - {isSelectedAssisted && ( -
-
- )} - {/* Unified whole-file diff with hunk overlays + notes sidebar */}
- {/* Avoid top padding here; it reads as a blank block between the controls and diff. */} -
- {props.isLoading && currentFileHunks.length === 0 ? ( -
- Loading diff... + {/* Diff column. The assisted-review banner lives INSIDE this column (not + above the whole body) so the agent's per-hunk comment spans only the + diff width and lines up with the code it refers to — rather than + stretching across the minimap and notes sidebar. */} +
+ {/* Assisted-review banner — surfaces the agent's flag + comment when + the selected hunk is one the agent pinned for review. Pinned to the + top of the diff column so the focus signal is impossible to miss + after entering immersive mode, where the side-panel cues aren't + visible. */} + {isSelectedAssisted && ( +
+
- ) : isReviewComplete ? ( -
-
-
-
-
-

Review complete

-

- You have already reviewed all {reviewedHunkLabel} in this diff. Return to chat - to keep going, or reopen reviewed hunks from the review panel if you want - another pass. -

-
- +
+
+
+

Review complete

+

+ You have already reviewed all {reviewedHunkLabel} in this diff. Return to chat + to keep going, or reopen reviewed hunks from the review panel if you want + another pass. +

+
+ +
-
- ) : currentFileHunks.length === 0 ? ( -
- {activeFilePath ? "No hunks for this file" : "No files to review"} -
- ) : ( -
- {isActiveFileRevealPending && ( -
- Loading file... + ) : currentFileHunks.length === 0 ? ( +
+ {activeFilePath ? "No hunks for this file" : "No files to review"} +
+ ) : ( +
+ {isActiveFileRevealPending && ( +
+ Loading file... +
+ )} +
+
- )} -
-
-
- )} + )} +
{!isReviewComplete && !isTouchExperience && ( From b2b4f83f45d1ead5dcb7ad0a42741f67bd6ad5fe Mon Sep 17 00:00:00 2001 From: Ammar Date: Mon, 1 Jun 2026 19:57:33 -0500 Subject: [PATCH 5/7] feat: lay out immersive review TODO as a horizontal strip Render the immersive review agent status bar's TODO plan as a single horizontally-scrolling row of compact chips instead of a tall vertical list, so the plan reserves minimal vertical space in the review viewport. - TodoList gains an optional `layout` prop ("vertical" default | "horizontal"). Horizontal mode lays chips in one scrollable row; the pinned list and tool history keep the default vertical layout untouched. - The bar drops its 240px-tall vertical scroll wrapper and renders the horizontal strip, keeping the collapse/persist toggle and streaming chip. --- src/browser/components/TodoList/TodoList.tsx | 30 ++++++++++++++++--- .../ImmersiveReviewAgentStatusBar.test.tsx | 4 +-- .../ImmersiveReviewAgentStatusBar.tsx | 9 +++--- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/browser/components/TodoList/TodoList.tsx b/src/browser/components/TodoList/TodoList.tsx index 2dd25907fd..3467d6965a 100644 --- a/src/browser/components/TodoList/TodoList.tsx +++ b/src/browser/components/TodoList/TodoList.tsx @@ -68,6 +68,13 @@ function calculateTextOpacity( interface TodoListProps { todos: TodoItem[]; + /** + * Orientation of the list: + * - "vertical" (default): stacked rows — used by the pinned list and tool history. + * - "horizontal": a single horizontally-scrolling row of compact chips, used by the + * immersive review status bar to keep the plan to one row of vertical space. + */ + layout?: "vertical" | "horizontal"; } function getStatusIcon(status: TodoItem["status"]): React.ReactNode { @@ -87,7 +94,8 @@ function getStatusIcon(status: TodoItem["status"]): React.ReactNode { * - TodoToolCall (in expanded tool history) * - PinnedTodoList (pinned at bottom of chat) */ -export const TodoList: React.FC = ({ todos }) => { +export const TodoList: React.FC = ({ todos, layout = "vertical" }) => { + const isHorizontal = layout === "horizontal"; // Count completed and pending items for fade effects const completedCount = todos.filter((t) => t.status === "completed").length; const pendingCount = todos.filter((t) => t.status === "pending").length; @@ -95,7 +103,16 @@ export const TodoList: React.FC = ({ todos }) => { let pendingIndex = 0; return ( -
+
{todos.map((todo, index) => { const currentCompletedIndex = todo.status === "completed" ? completedIndex++ : undefined; const currentPendingIndex = todo.status === "pending" ? pendingIndex++ : undefined; @@ -111,14 +128,19 @@ export const TodoList: React.FC = ({ todos }) => { return (
-
{getStatusIcon(todo.status)}
+
+ {getStatusIcon(todo.status)} +
{ test("renders the TODO plan (expanded) when todos exist", () => { seed("ws-todos", { todos }); const result = renderBar("ws-todos"); - // Vertical TodoList content is visible by default. + // The horizontal TodoList strip is visible by default. expect(result.getByText("Wire up status bar")).toBeTruthy(); expect(result.getByText("Add tests")).toBeTruthy(); // Summary reflects the counts. @@ -162,7 +162,7 @@ describe("ImmersiveReviewAgentStatusBar", () => { seed(workspaceId, { todos }); const first = renderBar(workspaceId); - // Plan is expanded by default; collapsing hides the vertical list. + // Plan is expanded by default; collapsing hides the horizontal strip. expect(first.getByText("Wire up status bar")).toBeTruthy(); fireEvent.click(first.getByRole("button", { name: /todo/i })); expect(first.queryByText("Wire up status bar")).toBeNull(); diff --git a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx index 6df1c9bfcd..216fd15c1c 100644 --- a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx +++ b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx @@ -6,7 +6,8 @@ * loses all signal about what the agent is doing. * * This bar restores that signal without leaving immersive: - * - the agent's TODO plan in a vertical layout (collapsible, persisted), and + * - the agent's TODO plan as a single horizontal strip (collapsible, + * persisted) so it reserves minimal review height, and * - live streaming status (starting / streaming / awaiting a question). * * Design notes: @@ -189,9 +190,9 @@ export const ImmersiveReviewAgentStatusBar: React.FC{trailingStatus}
)} {hasTodos && expanded && ( -
- -
+ // Horizontal strip: one row tall (the list scrolls sideways), so the + // plan costs minimal vertical space in the review viewport. + )}
); From 29e38ddd6674a5b6bf7f1b178b59e1f36fe867fd Mon Sep 17 00:00:00 2001 From: Ammar Date: Mon, 1 Jun 2026 20:03:41 -0500 Subject: [PATCH 6/7] feat: left-align streaming chip + add no-TODO Storybook coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the agent is streaming but hasn't written a TODO plan, the status bar previously pushed the "Streaming…" chip to the far right (ml-auto), leaving it floating alone on an otherwise-empty bar. Left-align the chip in the no-plan case so it reads as a status label; the TODO case keeps ml-auto so the chip + chevron stay right of the plan summary. Add an ImmersiveWithStreamingNoTodo Storybook story that seeds an open stream with no todo_write so Chromatic covers this chip-only state. The story seeding helper (seedAgentStatus) now skips the todo_write push when no todos are given; budget stays within limit (250/250, no bump). --- .../ImmersiveReviewAgentStatusBar.tsx | 16 +++- .../ImmersiveReviewView.stories.tsx | 93 ++++++++++++++----- 2 files changed, 80 insertions(+), 29 deletions(-) diff --git a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx index 216fd15c1c..4c334efd97 100644 --- a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx +++ b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx @@ -33,6 +33,7 @@ import { } from "@/browser/hooks/useWorkspaceStreamingStatusPhase"; import { usePersistedState } from "@/browser/hooks/usePersistedState"; import { getImmersiveReviewAgentBarExpandedKey } from "@/common/constants/storage"; +import { cn } from "@/common/lib/utils"; import type { TodoItem } from "@/common/types/tools"; /** The slice of WorkspaceState this bar reads. */ @@ -146,9 +147,12 @@ export const ImmersiveReviewAgentStatusBar: React.FC (
TODO {summaryParts.length > 0 && <> · {summaryParts.join(" · ")}} - {trailingStatus} + {renderStatusChip(true)} {expanded ? ( ) : ( @@ -187,7 +191,11 @@ export const ImmersiveReviewAgentStatusBar: React.FC ) : ( // Streaming/question only (no plan yet): static row, nothing to expand. -
{trailingStatus}
+ // Chip is left-aligned here so it reads as a status label rather than + // hugging the far right of an otherwise-empty bar. +
+ {renderStatusChip(false)} +
)} {hasTodos && expanded && ( // Horizontal strip: one row tall (the list scrolls sideways), so the diff --git a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.stories.tsx b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.stories.tsx index 95a86df38d..8481b727c3 100644 --- a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.stories.tsx +++ b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.stories.tsx @@ -269,12 +269,16 @@ function createImmersiveStoryClient(): APIClient { } interface AgentStatusSeed { - /** TODO plan the agent has written (drives the vertical TODO list + summary). */ - todos: TodoItem[]; + /** + * TODO plan the agent has written (drives the horizontal TODO strip + + * summary). Omit (or pass an empty array) to model "streaming before any plan + * is written", where the bar shows only the streaming chip. + */ + todos?: TodoItem[]; /** * When true, leaves the seeded stream open so the bar shows the live - * "Streaming…" chip alongside the plan. When false the stream is left - * un-started, so only the persisted (incomplete) plan shows. + * "Streaming…" chip. When false the stream is left un-started, so only the + * persisted (incomplete) plan shows. */ streaming?: boolean; } @@ -321,25 +325,29 @@ function seedAgentStatus( model: "anthropic:claude-sonnet-4", startTime: 0, }); - aggregator.handleToolCallStart({ - type: "tool-call-start", - workspaceId, - messageId, - toolCallId: `${workspaceId}-todo`, - toolName: "todo_write", - args: { todos: seed.todos }, - tokens: 10, - timestamp: 1, - }); - aggregator.handleToolCallEnd({ - type: "tool-call-end", - workspaceId, - messageId, - toolCallId: `${workspaceId}-todo`, - toolName: "todo_write", - result: { success: true }, - timestamp: 2, - }); + // Only push a plan when the seed has one; omitting it models "streaming + // before any TODO is written" so the bar renders the chip-only state. + if (seed.todos && seed.todos.length > 0) { + aggregator.handleToolCallStart({ + type: "tool-call-start", + workspaceId, + messageId, + toolCallId: `${workspaceId}-todo`, + toolName: "todo_write", + args: { todos: seed.todos }, + tokens: 10, + timestamp: 1, + }); + aggregator.handleToolCallEnd({ + type: "tool-call-end", + workspaceId, + messageId, + toolCallId: `${workspaceId}-todo`, + toolName: "todo_write", + result: { success: true }, + timestamp: 2, + }); + } if (seed.streaming !== true) { // Collapse the active stream so the chip clears; incomplete todos persist. aggregator.clearActiveStreams(); @@ -690,7 +698,7 @@ const IMMERSIVE_AGENT_STATUS_TODOS: TodoItem[] = [ /** * Immersive review with the top agent status bar populated: the agent's TODO - * plan in a vertical layout plus the live "Streaming…" chip. This is the piece + * plan as a horizontal strip plus the live "Streaming…" chip. This is the piece * that keeps chat status visible while the user reviews code behind the * full-screen overlay. Seeds the WorkspaceStore so the bar has real state. */ @@ -708,7 +716,7 @@ export const ImmersiveWithAgentStatusBar: Story = { await waitFor( () => { canvas.getByTestId("immersive-review-view"); - // The collapsible TODO bar + its vertical plan content render. + // The collapsible TODO bar + its horizontal plan strip render. canvas.getByTestId("immersive-agent-status-bar"); canvas.getByText("Add the top status bar (TODO + streaming)"); // Live streaming chip is visible alongside the plan. @@ -718,3 +726,38 @@ export const ImmersiveWithAgentStatusBar: Story = { ); }, }; + +const IMMERSIVE_STREAMING_NO_TODO_WORKSPACE_ID = "ws-review-immersive-streaming-no-todo"; + +/** + * Immersive review while the agent is streaming but has not written a TODO plan + * yet. Locks in the chip-only state of the status bar: with no plan on the left + * the "Streaming…" chip is left-aligned (reads as a status label) instead of + * floating alone on the far right of an otherwise-empty bar, and the bar stays + * a single row tall. Seeds an open stream with no `todo_write` call. + */ +export const ImmersiveWithStreamingNoTodo: Story = { + render: () => ( + + ), + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + + await waitFor( + () => { + canvas.getByTestId("immersive-review-view"); + // The bar renders the streaming chip with no TODO toggle/summary. + canvas.getByTestId("immersive-agent-status-bar"); + canvas.getByText("Streaming…"); + if (canvas.queryByText("TODO")) { + throw new Error("Expected the chip-only status bar with no TODO plan."); + } + }, + { timeout: 10_000 } + ); + }, +}; From ea9cea88a7fafe5767a5a627fee087e65aae7c00 Mon Sep 17 00:00:00 2001 From: Ammar Date: Mon, 1 Jun 2026 20:09:50 -0500 Subject: [PATCH 7/7] perf: subscribe immersive status bar to individual workspace fields Codex P2: the bar's useSyncExternalStore returned the whole getWorkspaceState() object as its snapshot. Because that object is version-cached, its reference changes on every WorkspaceState bump (e.g. each streamed message), re-rendering the bar on every token while the user reviews code. Read each field the bar uses (todos, canInterrupt, isStreamStarting, awaitingUserQuestion) as its own snapshot instead. Primitives compare by value and todos keeps a stable aggregator reference (same basis as PinnedTodoList), so the bar only re-renders when a field it actually uses changes. Add a regression test asserting the bar ignores unrelated state bumps and re-renders only on a watched-field change. --- .../ImmersiveReviewAgentStatusBar.test.tsx | 63 ++++++++++++++++- .../ImmersiveReviewAgentStatusBar.tsx | 70 +++++++++---------- 2 files changed, 96 insertions(+), 37 deletions(-) diff --git a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.test.tsx b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.test.tsx index 1431673fa3..fce97dc147 100644 --- a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.test.tsx +++ b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.test.tsx @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, mock, spyOn, test } from "bun:test"; -import { cleanup, fireEvent, render, type RenderResult } from "@testing-library/react"; +import { act, cleanup, fireEvent, render, type RenderResult } from "@testing-library/react"; +import { Profiler } from "react"; import { installDom } from "../../../../../tests/ui/dom"; // Import the module namespace (not the hook directly) so we can spyOn the hook @@ -68,6 +69,31 @@ function seed(workspaceId: string, input: SeedInput): void { seeds.set(workspaceId, buildState(workspaceId, input)); } +function notify(workspaceId: string): void { + getSubscribers(workspaceId).forEach((cb) => cb()); +} + +/** + * Replace the cached state with a NEW object reference whose watched fields + * (todos/canInterrupt/isStreamStarting/awaitingUserQuestion) are byte-identical + * (todos keeps the same array ref), then notify subscribers — models an + * unrelated WorkspaceState bump such as a streamed message arriving. + */ +function bumpUnrelated(workspaceId: string): void { + const prev = seeds.get(workspaceId); + if (!prev) throw new Error(`Missing seed for ${workspaceId}`); + seeds.set(workspaceId, { ...prev, name: `${prev.name}-bump`, messages: [...prev.messages] }); + notify(workspaceId); +} + +/** Patch a watched field on the cached state and notify subscribers. */ +function patchState(workspaceId: string, patch: Partial): void { + const prev = seeds.get(workspaceId); + if (!prev) throw new Error(`Missing seed for ${workspaceId}`); + seeds.set(workspaceId, { ...prev, ...patch }); + notify(workspaceId); +} + // Minimal fake exposing only the store methods the bar calls. Cast through // unknown because the bar uses just this slice of the WorkspaceStore surface. const fakeStore = { @@ -179,4 +205,39 @@ describe("ImmersiveReviewAgentStatusBar", () => { fireEvent.click(second.getByRole("button", { name: /todo/i })); expect(second.getByText("Wire up status bar")).toBeTruthy(); }); + + test("does not re-render on unrelated workspace-state bumps, only on watched fields", () => { + const workspaceId = "ws-stable"; + seed(workspaceId, { todos, canInterrupt: true }); + + let commits = 0; + const result = render( + { + commits += 1; + }} + > + + + ); + expect(result.getByText("Streaming…")).toBeTruthy(); + + // An unrelated state bump (new WorkspaceState ref, same watched fields) + // must NOT re-render the bar — this is the leaf-subscription guarantee that + // keeps streamed-message churn off the immersive diff's sibling bar. + const committedAfterMount = commits; + act(() => { + bumpUnrelated(workspaceId); + bumpUnrelated(workspaceId); + }); + expect(commits).toBe(committedAfterMount); + + // Changing a field the bar actually reads DOES re-render it. + act(() => { + patchState(workspaceId, { awaitingUserQuestion: true }); + }); + expect(commits).toBeGreaterThan(committedAfterMount); + expect(result.getByText("Mux has a question")).toBeTruthy(); + }); }); diff --git a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx index 4c334efd97..9ba9b50aa2 100644 --- a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx +++ b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewAgentStatusBar.tsx @@ -36,24 +36,10 @@ import { getImmersiveReviewAgentBarExpandedKey } from "@/common/constants/storag import { cn } from "@/common/lib/utils"; import type { TodoItem } from "@/common/types/tools"; -/** The slice of WorkspaceState this bar reads. */ -interface AgentStatusSnapshot { - todos: TodoItem[]; - canInterrupt: boolean; - isStreamStarting: boolean; - awaitingUserQuestion: boolean; -} - -// Stable idle snapshot returned when the workspace isn't registered yet (tests, -// storybook, teardown). A module-level constant keeps the useSyncExternalStore -// snapshot referentially stable so the store's "getSnapshot should be cached" -// guard doesn't loop, and the bar simply renders nothing. -const IDLE_SNAPSHOT: AgentStatusSnapshot = { - todos: [], - canInterrupt: false, - isStreamStarting: false, - awaitingUserQuestion: false, -}; +// Stable empty-plan reference for the unregistered case (tests, storybook, +// teardown). Module-level so the `todos` snapshot stays referentially stable +// and useSyncExternalStore's "getSnapshot should be cached" guard doesn't loop. +const EMPTY_TODOS: TodoItem[] = []; interface ImmersiveReviewAgentStatusBarProps { workspaceId: string; @@ -67,27 +53,39 @@ export const ImmersiveReviewAgentStatusBar: React.FC( - (callback) => - workspaceStore.hasRegisteredWorkspace(workspaceId) - ? workspaceStore.subscribeKey(workspaceId, callback) - : () => undefined, - () => - workspaceStore.hasRegisteredWorkspace(workspaceId) - ? workspaceStore.getWorkspaceState(workspaceId) - : IDLE_SNAPSHOT + const subscribe = (callback: () => void) => + workspaceStore.hasRegisteredWorkspace(workspaceId) + ? workspaceStore.subscribeKey(workspaceId, callback) + : () => undefined; + const todos = useSyncExternalStore(subscribe, () => + workspaceStore.hasRegisteredWorkspace(workspaceId) + ? workspaceStore.getWorkspaceState(workspaceId).todos + : EMPTY_TODOS + ); + const canInterrupt = useSyncExternalStore(subscribe, () => + workspaceStore.hasRegisteredWorkspace(workspaceId) + ? workspaceStore.getWorkspaceState(workspaceId).canInterrupt + : false ); - const todos = snapshot.todos; - const canInterrupt = snapshot.canInterrupt; // Sidebar derives `isStarting` directly from `isStreamStarting`. - const isStarting = snapshot.isStreamStarting; - const awaitingUserQuestion = snapshot.awaitingUserQuestion; + const isStarting = useSyncExternalStore(subscribe, () => + workspaceStore.hasRegisteredWorkspace(workspaceId) + ? workspaceStore.getWorkspaceState(workspaceId).isStreamStarting + : false + ); + const awaitingUserQuestion = useSyncExternalStore(subscribe, () => + workspaceStore.hasRegisteredWorkspace(workspaceId) + ? workspaceStore.getWorkspaceState(workspaceId).awaitingUserQuestion + : false + ); // Held phase keeps the streaming chip steady across the starting->streaming // handoff so it doesn't blink out for a frame between adjacent state settles.