From 9659e3e450fbd0d67ac756aa339082fa26bea4ab Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Fri, 21 Aug 2026 22:30:46 +0200 Subject: [PATCH 1/6] Keep the thread panel host mounted across thread navigation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thread-to-thread navigation remounted the SecondaryPanelLayout PanelGroup because the group key defaulted to resetKey (the thread id). On desktop-inline layouts that destroyed and rebuilt the realized secondary panel (diff views, metadata, browser deck) in one synchronous commit per navigate, and reset the user's panel sizes every time. Pass the documented panelGroupKey escape hatch (already used by PluginPanelRightPanelHost) so the physical host survives navigation while content identity still resets via resetKey. Scope, stated plainly: this is a desktop-inline improvement. On compact viewports the drawer already rendered outside the keyed group, and the timeline/composer remount cost on any viewport is owned by PageShell's own key={threadId} inside EmbeddedThreadChat — unchanged here. Per-thread state stays correct: drafts and scroll anchors live under that PageShell key, split layouts are keyed by thread id, and the hasPanelExpandedRef mount-collapse guard now re-arms per thread since the Panel instance survives navigation. Co-Authored-By: Claude Fable 5 --- .../secondary-panel/ThreadSecondaryPanel.tsx | 9 ++++ .../ThreadDetailSecondaryContent.test.tsx | 43 +++++++++++++++++++ .../ThreadDetailSecondaryContent.tsx | 5 +++ 3 files changed, 57 insertions(+) diff --git a/apps/app/src/components/secondary-panel/ThreadSecondaryPanel.tsx b/apps/app/src/components/secondary-panel/ThreadSecondaryPanel.tsx index 78cc4260a2..ae071bda39 100644 --- a/apps/app/src/components/secondary-panel/ThreadSecondaryPanel.tsx +++ b/apps/app/src/components/secondary-panel/ThreadSecondaryPanel.tsx @@ -6,6 +6,7 @@ import { type TransitionEvent, useCallback, useContext, + useLayoutEffect, useMemo, useRef, useState, @@ -385,6 +386,14 @@ export function ThreadSecondaryPanel({ // silently closing it again. Only a collapse from a layout this Panel // instance actually held expanded may close the persisted panel. const hasPanelExpandedRef = useRef(false); + // The panel host survives thread navigation (stable panelGroupKey on the + // thread-detail layout), so the guard must re-arm per thread: a collapse + // applied while showing the next thread must not pass on the previous + // thread's expansion. Child layout effects run before the parent group's + // setLayout effect, so this reset lands first. + useLayoutEffect(() => { + hasPanelExpandedRef.current = false; + }, [splitPanelStateId]); const handlePanelResize = useCallback( (size: number) => { if (size > 0) { diff --git a/apps/app/src/views/thread-detail/ThreadDetailSecondaryContent.test.tsx b/apps/app/src/views/thread-detail/ThreadDetailSecondaryContent.test.tsx index 207a7af319..bfc18d5732 100644 --- a/apps/app/src/views/thread-detail/ThreadDetailSecondaryContent.test.tsx +++ b/apps/app/src/views/thread-detail/ThreadDetailSecondaryContent.test.tsx @@ -422,6 +422,49 @@ describe("ThreadDetailSecondaryContent", () => { }); }); + it("keeps the panel subtree mounted when navigating between threads", async () => { + const props = createProps(); + const { rerender } = render( + + + + + + + , + ); + + const sidePanel = await screen.findByTestId( + "inline-secondary-panel", + {}, + { timeout: 5_000 }, + ); + const panelGroup = screen.getByTestId("panel-group"); + + const nextProps = createProps(); + nextProps.timeline = { + ...nextProps.timeline, + threadId: "thread-2", + } as ThreadDetailSecondaryContentProps["timeline"]; + rerender( + + + + + + + , + ); + + // Navigation swaps content identity but must not remount the physical + // panel host: same DOM nodes for the group and the realized side panel. + expect(screen.getByTestId("panel-group")).toBe(panelGroup); + expect(screen.getByTestId("inline-secondary-panel")).toBe(sidePanel); + expect( + screen.getByTestId("thread-timeline-pane").getAttribute("data-thread-id"), + ).toBe("thread-2"); + }); + it("only requests the forks list while the secondary panel is open", () => { const props = createProps(); const { rerender } = render( diff --git a/apps/app/src/views/thread-detail/ThreadDetailSecondaryContent.tsx b/apps/app/src/views/thread-detail/ThreadDetailSecondaryContent.tsx index 6c7551cd73..bf2683ca2e 100644 --- a/apps/app/src/views/thread-detail/ThreadDetailSecondaryContent.tsx +++ b/apps/app/src/views/thread-detail/ThreadDetailSecondaryContent.tsx @@ -125,6 +125,11 @@ function ThreadDetailSecondaryContentBody({ open={isSecondaryPanelOpen} onToggle={onToggleSecondaryPanel} onClose={threadSecondaryPanelProps.onClose} + // The physical panel host survives thread-to-thread navigation; only + // content identity (resetKey) changes. Per-thread state below is safe: + // the timeline, composer and scroll anchors live under PageShell's own + // key={threadId} inside EmbeddedThreadChat. + panelGroupKey="thread-detail" resetKey={timeline.threadId} contentKey={timeline.threadId} drawerLabel="Thread details" From 1844e58b5ab87715ca9107cca248575d6b414089 Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Fri, 21 Aug 2026 22:37:31 +0200 Subject: [PATCH 2/6] Gate the default focus refetch on lost realtime coverage Every window focus refetched every active query older than 2s. On a phone that meant each unlock and app switch fired a full refetch wave on top of the realtime reconnect wave: the WebSocket manager already probes the socket on visible, the reconnect watermark refetches exactly the queries whose data predates the disconnect, and changes merged while hidden flush on the next visible. The focus wave duplicated all of that in the first interactive frames after unlock. Make the default focus refetch injectable and gate it in main on wsManager's connection state: while the state is "connected", realtime owns freshness; in "connecting" or "reconnecting" the focus refetch remains the fallback. Per-query refetchOnWindowFocus policies (query-policies.ts) are unaffected and still win. Known narrow trade, called out for review: a half-open socket reports "connected" for up to the 5s pong timeout, and the handful of queries with neither realtime coverage nor a focus policy (e.g. CLI skills status) skip one focus wave when parked across an unlock; refetchOnMount still repairs them on navigation. Co-Authored-By: Claude Fable 5 --- apps/app/src/lib/query-client.test.ts | 77 +++++++++++++++++++++++++++ apps/app/src/lib/query-client.ts | 17 +++++- apps/app/src/main.tsx | 9 +++- 3 files changed, 101 insertions(+), 2 deletions(-) diff --git a/apps/app/src/lib/query-client.test.ts b/apps/app/src/lib/query-client.test.ts index 57adcac03e..66b1b187da 100644 --- a/apps/app/src/lib/query-client.test.ts +++ b/apps/app/src/lib/query-client.test.ts @@ -117,6 +117,83 @@ describe("createAppQueryClient", () => { queryClient.clear(); }); + it("keeps the default focus refetch when no gate is configured", async () => { + const queryClient = createAppQueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + showMutationErrorToasts: false, + }); + queryClient.mount(); + + const queryFn = vi.fn(() => Promise.resolve("data")); + const observer = new QueryObserver(queryClient, { + queryKey: ["focus-ungated"], + queryFn, + staleTime: 0, + }); + const unsubscribe = observer.subscribe(() => {}); + + await vi.waitFor(() => { + expect(observer.getCurrentResult().data).toBe("data"); + }); + + window.dispatchEvent(new Event("pageshow")); + await vi.waitFor(() => { + expect(queryFn).toHaveBeenCalledTimes(2); + }); + + unsubscribe(); + queryClient.unmount(); + queryClient.clear(); + }); + + it("skips the default focus refetch while the gate reports realtime coverage", async () => { + let realtimeConnected = true; + const queryClient = createAppQueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + shouldRefetchOnWindowFocus: () => !realtimeConnected, + showMutationErrorToasts: false, + }); + queryClient.mount(); + + const queryFn = vi.fn(() => Promise.resolve("data")); + const observer = new QueryObserver(queryClient, { + queryKey: ["focus-gated"], + queryFn, + // Instantly stale so a permitted focus refetch always fires. + staleTime: 0, + }); + const unsubscribe = observer.subscribe(() => {}); + + await vi.waitFor(() => { + expect(observer.getCurrentResult().data).toBe("data"); + }); + expect(queryFn).toHaveBeenCalledTimes(1); + + // Connected: realtime owns freshness, focus must not refetch. + window.dispatchEvent(new Event("pageshow")); + await Promise.resolve(); + expect(queryFn).toHaveBeenCalledTimes(1); + + // Coverage lost: focus refetch is the fallback again. + realtimeConnected = false; + window.dispatchEvent(new Event("pageshow")); + await vi.waitFor(() => { + expect(queryFn).toHaveBeenCalledTimes(2); + }); + + unsubscribe(); + queryClient.unmount(); + queryClient.clear(); + }); + it("resumes a suspend-cancelled fetch that no focus refetch would restart", async () => { const queryClient = createAppQueryClient({ defaultOptions: { diff --git a/apps/app/src/lib/query-client.ts b/apps/app/src/lib/query-client.ts index 0b731ab3fc..5f7c905576 100644 --- a/apps/app/src/lib/query-client.ts +++ b/apps/app/src/lib/query-client.ts @@ -17,6 +17,17 @@ import { interface CreateAppQueryClientOptions { defaultOptions?: QueryClientConfig["defaultOptions"]; showMutationErrorToasts?: boolean; + /** + * Gate for the default focus refetch. Focus refetch is the freshness + * fallback for when realtime coverage is lost; while the socket is + * connected, change events keep the cache correct and the reconnect + * watermark repairs any gap, so a focus event (every phone unlock and + * app switch) must not refetch every active query on top of that wave. + * Defaults to always refetching. A `defaultOptions.queries.refetchOnWindowFocus` + * passed alongside this gate wins over it (caller defaults are spread last), + * so pass one or the other. + */ + shouldRefetchOnWindowFocus?: () => boolean; } interface AppQueryClientBrowserEventCleanup { @@ -108,6 +119,7 @@ export function createAppQueryClient( const defaultOptions = options.defaultOptions; const showMutationErrorToasts = options.showMutationErrorToasts ?? true; + const shouldRefetchOnWindowFocus = options.shouldRefetchOnWindowFocus; return new QueryClient({ mutationCache: new MutationCache({ @@ -133,7 +145,10 @@ export function createAppQueryClient( ...defaultOptions, queries: { staleTime: 2000, - refetchOnWindowFocus: true, + refetchOnWindowFocus: + shouldRefetchOnWindowFocus === undefined + ? true + : () => shouldRefetchOnWindowFocus(), retry: shouldRetryTransientReadQuery, retryDelay: TRANSIENT_READ_RETRY_DELAY_MS, ...defaultOptions?.queries, diff --git a/apps/app/src/main.tsx b/apps/app/src/main.tsx index 1eba2c5ec6..4d1c8fd865 100644 --- a/apps/app/src/main.tsx +++ b/apps/app/src/main.tsx @@ -15,6 +15,7 @@ import { installAppQueryClientBrowserEvents, } from "./lib/query-client"; import { applyCachedAppThemeCss } from "./lib/themes"; +import { wsManager } from "./lib/ws"; import "./app.css"; // Before anything renders: a content script that moves a React-owned node out @@ -28,7 +29,13 @@ installForeignDomMutationGuard(); // costs anything when an Error is actually constructed. Error.stackTraceLimit = 50; -const queryClient = createAppQueryClient(); +const queryClient = createAppQueryClient({ + // While the realtime socket is connected, change events and the reconnect + // watermark own cache freshness; a focus refetch on top would re-request + // every active query on each phone unlock and app switch. + shouldRefetchOnWindowFocus: () => + wsManager.getConnectionState() !== "connected", +}); installAppQueryClientBrowserEvents(queryClient); // The provider CLI install store outlives every component, so it takes the // client here rather than reading it from context when an install finishes. From 699b7cae3c0c4e12d8a391601398b9854b730ba5 Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Fri, 21 Aug 2026 22:54:45 +0200 Subject: [PATCH 3/6] Make the published plugin composer host stable across keystrokes Root cause (mobile telemetry: 19.6s frame hang after keydown in the composer): ThreadDetailPromptArea's published PluginComposerHost embedded the live draft as a value field (draft: currentPromptDraft), so every keystroke built a new host object. usePublishPluginComposerHost's identity check notified the pane scope, ThreadDetailSecondaryContentBody (usePluginComposerHost) re-rendered SecondaryPanelLayout -> ThreadTimelinePane (fresh element per render) -> PageShell -> ThreadTimelineSurface per character, and SecondaryPanelLayout republished its hosted-panel model to the workspace host per character. Change: PluginComposerHost drops the draft value field; the live draft is exposed as getCurrent() + subscribeDraft() and read through the new usePluginComposerHostDraft hook, so only actual draft consumers re-render. - usePromptDraftStorage and getPromptDraftAccessor gain a stable per-key subscribe; the thread and new-thread hosts pass it through and become identity-stable per thread/project. - Hosts whose draft lives in React state (inline queued-message and sent-message editors, EmbeddedThreadChat's pair) get subscribeDraft from useComposerHostDraftNotifier, which notifies from a layout effect once a render committed a different draft. In EmbeddedThreadChat the notifiers are declared after ALL ref-sync layout effects, including the active-identity syncs: a thread switch changes the host identity and the draft in one commit, and useSyncExternalStore reads the snapshot inside the notification, so notifying before the identity sync would hand every subscriber the stale pre-switch draft with no later notification to correct it. - The queued-message and sent-message hosts are keyed on their session scalars (editSessionId/operationId), so every host in the app now honors the identity-stable contract; the published value only flips between two stable identities (thread host <-> session host). EmbeddedThreadChat's ...WithDraft wrapper memos are deleted. - Draft readers move to the hook: useComposerView/useComposer (plugin SDK contract unchanged - same values, same update timing) and FollowUpPromptBoxStackOnly. Tests: ThreadDetailPromptArea.keystrokes.test.tsx uses the real draft store and asserts that 21 keystrokes cause zero re-renders of a shell probe holding the published host while a subscribed consumer tracks every character; that submit reads the draft imperatively at event time; that external store writes reach consumers while a pending interaction hides the composer; and that inline queued edits publish one per-session host, stream keystrokes without shell renders, and restore the identical thread host on close. ThreadDetailSecondaryContent.test.tsx mounts the real body/SecondaryPanelLayout with a store-backed publisher in the footer slot and asserts the timeline pane render count stays flat across 20 draft writes. EmbeddedThreadChat.test.tsx switches threads with pre-seeded drafts and asserts subscribers observe the new thread's draft at the notification itself (fails when the notifiers run before the identity sync). All of these fail against the previous per-keystroke host identity. Co-Authored-By: Claude Fable 5 --- .../plugin/ComposerExtensionHost.test.tsx | 6 +- .../plugin/PluginComposerActions.stories.tsx | 6 +- .../plugin/plugin-composer-host.tsx | 66 +- .../plugin/plugin-slot-mounts.test.tsx | 22 +- .../promptbox/FollowUpPromptBox.test.tsx | 6 +- .../promptbox/FollowUpPromptBox.tsx | 6 +- .../promptbox/NewThreadComposer.tsx | 6 +- .../promptbox/PromptBoxActionsMenu.test.tsx | 4 +- .../promptbox/PromptBoxInternal.test.tsx | 8 +- .../embedded-chat/EmbeddedThreadChat.test.tsx | 178 +++++- .../embedded-chat/EmbeddedThreadChat.tsx | 45 +- apps/app/src/hooks/usePromptDraftStorage.ts | 11 + apps/app/src/lib/plugin-sdk-hooks.ts | 7 +- .../thread-detail/SplitThreadArea.test.tsx | 2 +- ...ThreadDetailPromptArea.keystrokes.test.tsx | 582 ++++++++++++++++++ .../ThreadDetailPromptArea.test.tsx | 3 +- .../thread-detail/ThreadDetailPromptArea.tsx | 183 ++++-- .../ThreadDetailSecondaryContent.test.tsx | 105 +++- 18 files changed, 1100 insertions(+), 146 deletions(-) create mode 100644 apps/app/src/views/thread-detail/ThreadDetailPromptArea.keystrokes.test.tsx diff --git a/apps/app/src/components/plugin/ComposerExtensionHost.test.tsx b/apps/app/src/components/plugin/ComposerExtensionHost.test.tsx index 557130e455..ddcc038559 100644 --- a/apps/app/src/components/plugin/ComposerExtensionHost.test.tsx +++ b/apps/app/src/components/plugin/ComposerExtensionHost.test.tsx @@ -6,6 +6,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { AppCommandProvider } from "@/components/commands/AppCommandProvider"; import { usePluginComposerHost, + usePluginComposerHostDraft, useOptionalPluginComposerView, type PluginComposerHost, } from "./plugin-composer-host"; @@ -52,11 +53,12 @@ const draft = { text: "hello", mentions: [], attachments: [] }; function RendererProbe() { const host = usePluginComposerHost(); + const hostDraft = usePluginComposerHostDraft(host); const view = useOptionalPluginComposerView(); return (
); @@ -74,9 +76,9 @@ function Harness({ const host = useMemo( () => ({ scope: { kind: "thread", threadId: "thr_test" }, - draft, textEffectKey: "thread/thr_test", getCurrent: () => draft, + subscribeDraft: () => () => {}, setDraft: () => undefined, focus: mocks.focusHost, }), diff --git a/apps/app/src/components/plugin/PluginComposerActions.stories.tsx b/apps/app/src/components/plugin/PluginComposerActions.stories.tsx index e77ee0850b..fbb92b9398 100644 --- a/apps/app/src/components/plugin/PluginComposerActions.stories.tsx +++ b/apps/app/src/components/plugin/PluginComposerActions.stories.tsx @@ -20,6 +20,7 @@ import { SidebarMenu, SidebarMenuItem } from "@/components/ui/sidebar"; import { PromptBoxInternal } from "@/components/promptbox/PromptBoxInternal"; import { PluginComposerHostProvider, + useComposerHostDraftNotifier, type PluginComposerHost, } from "@/components/plugin/plugin-composer-host"; import { @@ -237,16 +238,17 @@ function ThreadRowStatusFixture() { }); const draftRef = useRef(draft); draftRef.current = draft; + const subscribeDraft = useComposerHostDraftNotifier(draft); const composerHost = useMemo( () => ({ scope: { kind: "thread", threadId: THREAD_ID }, - draft, textEffectKey: `story:${THREAD_ID}`, getCurrent: () => draftRef.current, + subscribeDraft, setDraft, focus: () => {}, }), - [draft], + [subscribeDraft], ); const [queryClient] = useState( () => diff --git a/apps/app/src/components/plugin/plugin-composer-host.tsx b/apps/app/src/components/plugin/plugin-composer-host.tsx index 70ffa3ec61..c1220122f5 100644 --- a/apps/app/src/components/plugin/plugin-composer-host.tsx +++ b/apps/app/src/components/plugin/plugin-composer-host.tsx @@ -5,6 +5,7 @@ import { useEffect, useLayoutEffect, useMemo, + useRef, useState, useSyncExternalStore, type ReactNode, @@ -18,12 +19,23 @@ import type { PromptDraftState } from "@bb/client-core"; * authoritative even when the route points at another split pane and also * carries host-only state such as root-project selection or an inline queued * message editor. + * + * A host must stay referentially stable while the user types: it is published + * to the pane scope (and provided via context) where large non-draft + * subscribers such as the secondary-panel layout hold it, so a per-keystroke + * identity would re-render the whole thread shell per character. The live + * draft is therefore exposed as `getCurrent` + `subscribeDraft` instead of a + * value field; draft consumers read it via `usePluginComposerHostDraft`. */ export interface PluginComposerHost { scope: PluginComposerScope; - draft: PromptDraftState; textEffectKey: string; getCurrent(): PromptDraftState; + /** + * Subscribes to changes of `getCurrent()`'s committed result. Returns an + * unsubscribe function. Must be identity-stable for the host's lifetime. + */ + subscribeDraft(listener: () => void): () => void; setDraft(next: PromptDraftState): void; focus(): void; } @@ -41,6 +53,58 @@ export function composerScopeIdentity(scope: PluginComposerScope): string { } } +const subscribeToNoDraft = () => () => {}; +const getNoDraft = () => null; + +/** + * The live draft of a composer host. This is the only reactive read of a + * host's draft: the host object itself stays identity-stable across + * keystrokes, so components that hold a host without calling this hook do not + * re-render while the user types. + */ +export function usePluginComposerHostDraft( + host: PluginComposerHost | null, +): PromptDraftState | null { + const subscribe = host?.subscribeDraft ?? subscribeToNoDraft; + const getSnapshot = host?.getCurrent ?? getNoDraft; + return useSyncExternalStore(subscribe, getSnapshot, getSnapshot); +} + +/** + * `subscribeDraft` for hosts whose draft lives in React state (the inline + * queued-message and sent-message editors) rather than in the prompt-draft + * store. Returns a stable subscribe function; listeners fire in a layout + * effect after a render committed a different `draft` identity, by which point + * the host's ref-backed `getCurrent` already returns the new value (edit + * commits write their ref synchronously, `useLatestRef` writes during render). + * Pass null while the corresponding editor is closed. + */ +export function useComposerHostDraftNotifier( + draft: PromptDraftState | null, +): (listener: () => void) => () => void { + const [store] = useState(() => { + const listeners = new Set<() => void>(); + return { + subscribe: (listener: () => void) => { + listeners.add(listener); + return () => { + listeners.delete(listener); + }; + }, + notify: () => { + for (const listener of [...listeners]) listener(); + }, + }; + }); + const previousDraftRef = useRef(draft); + useLayoutEffect(() => { + if (previousDraftRef.current === draft) return; + previousDraftRef.current = draft; + store.notify(); + }, [draft, store]); + return store.subscribe; +} + interface PluginComposerViewModelInput { scope: PluginComposerScope; layout: ComposerView["layout"]; diff --git a/apps/app/src/components/plugin/plugin-slot-mounts.test.tsx b/apps/app/src/components/plugin/plugin-slot-mounts.test.tsx index 44e97f8fd9..de81b14f28 100644 --- a/apps/app/src/components/plugin/plugin-slot-mounts.test.tsx +++ b/apps/app/src/components/plugin/plugin-slot-mounts.test.tsx @@ -48,6 +48,7 @@ import { PluginComposerHostProvider, PluginComposerHostScopeProvider, type PluginComposerHost, + useComposerHostDraftNotifier, usePublishPluginComposerHost, } from "./plugin-composer-host"; import { PluginHomepageSections } from "./PluginHomepageSections"; @@ -512,6 +513,7 @@ describe("useComposer", () => { }); const draftRef = useRef(draft); draftRef.current = draft; + const subscribeDraft = useComposerHostDraftNotifier(draft); const host = useMemo( () => ({ scope: { @@ -519,13 +521,13 @@ describe("useComposer", () => { threadId: "thr_queue", queuedMessageId, }, - draft, textEffectKey: `queued-message:thr_queue:${queuedMessageId}:1`, getCurrent: () => draftRef.current, + subscribeDraft, setDraft, focus: () => {}, }), - [draft, queuedMessageId], + [queuedMessageId, subscribeDraft], ); return ( @@ -616,10 +618,10 @@ describe("useComposer", () => { threadId: "thr_queue", queuedMessageId: "qmsg_1", }, - draft, textEffectKey: "queued-message:thr_queue:qmsg_1:sibling-surface", getCurrent: () => draft, + subscribeDraft: () => () => {}, setDraft, focus: () => {}, } @@ -683,6 +685,7 @@ describe("useComposer", () => { }); const draftRef = useRef(draft); draftRef.current = draft; + const subscribeDraft = useComposerHostDraftNotifier(draft); const host = useMemo( () => ({ scope: { @@ -692,13 +695,13 @@ describe("useComposer", () => { tabId: "side-chat:one", childThreadId, }, - draft, textEffectKey: `side-chat:side-chat:one:${childThreadId ?? ""}`, getCurrent: () => draftRef.current, + subscribeDraft, setDraft, focus: () => {}, }), - [childThreadId, draft], + [childThreadId, subscribeDraft], ); return ( @@ -815,16 +818,17 @@ describe("useComposer", () => { }); const draftRef = useRef(draft); draftRef.current = draft; + const subscribeDraft = useComposerHostDraftNotifier(draft); const host = useMemo( () => ({ scope: { kind: "new-thread", projectId }, - draft, textEffectKey: `root:${projectId}`, getCurrent: () => draftRef.current, + subscribeDraft, setDraft, focus: () => {}, }), - [draft, projectId], + [projectId, subscribeDraft], ); return ( @@ -901,9 +905,9 @@ describe("useComposer", () => { }; return { scope: { kind: "new-thread", projectId }, - draft, textEffectKey: `root-state:${projectId ?? "null"}`, getCurrent: () => draft, + subscribeDraft: () => () => {}, setDraft: () => {}, focus: () => {}, }; @@ -1050,11 +1054,11 @@ describe("useComposer", () => { threadId: "thr_scope_owner", queuedMessageId, }, - draft, // A host can retain its editable surface while its logical scope // changes, as root compose does when the selected project changes. textEffectKey: "shared-scope-effect", getCurrent: () => draft, + subscribeDraft: () => () => {}, setDraft: () => {}, focus: () => {}, }), diff --git a/apps/app/src/components/promptbox/FollowUpPromptBox.test.tsx b/apps/app/src/components/promptbox/FollowUpPromptBox.test.tsx index 6bf26d9245..b2ec85bebe 100644 --- a/apps/app/src/components/promptbox/FollowUpPromptBox.test.tsx +++ b/apps/app/src/components/promptbox/FollowUpPromptBox.test.tsx @@ -361,9 +361,9 @@ describe("FollowUpPromptBox", () => { stack={<>} pluginComposerHost={{ scope: { kind: "thread", threadId: "thr_test" }, - draft, textEffectKey: "thread:thr_test", getCurrent: () => draft, + subscribeDraft: () => () => {}, setDraft: vi.fn(), focus: vi.fn(), }} @@ -430,9 +430,9 @@ describe("FollowUpPromptBox", () => { stack={
Queued messages
} pluginComposerHost={{ scope: { kind: "thread", threadId: "thr_test" }, - draft, textEffectKey: "thread:thr_test", getCurrent: () => draft, + subscribeDraft: () => () => {}, setDraft: vi.fn(), focus: vi.fn(), }} @@ -611,9 +611,9 @@ describe("FollowUpPromptBox", () => { isPrimaryComposer={isPrimaryComposer} pluginComposerHost={{ scope, - draft, textEffectKey: "queued:queued_1", getCurrent: () => draft, + subscribeDraft: () => () => {}, setDraft: vi.fn(), focus: vi.fn(), }} diff --git a/apps/app/src/components/promptbox/FollowUpPromptBox.tsx b/apps/app/src/components/promptbox/FollowUpPromptBox.tsx index 80bb9b76cf..7bd8871707 100644 --- a/apps/app/src/components/promptbox/FollowUpPromptBox.tsx +++ b/apps/app/src/components/promptbox/FollowUpPromptBox.tsx @@ -25,6 +25,7 @@ import { PluginComposerHostProvider, PluginComposerViewProvider, type PluginComposerHost, + usePluginComposerHostDraft, usePluginComposerViewModel, } from "@/components/plugin/plugin-composer-host"; import { @@ -262,11 +263,12 @@ function FollowUpPromptBoxStackOnly({ >) { const composerScope = pluginComposerScope ?? pluginComposerHost?.scope ?? null; + const hostDraft = usePluginComposerHostDraft(pluginComposerHost ?? null); const composerView = usePluginComposerViewModel({ scope: composerScope ?? DEFAULT_FOLLOW_UP_COMPOSER_SCOPE, layout: "expanded", - text: pluginComposerHost?.draft.text ?? "", - attachmentCount: pluginComposerHost?.draft.attachments.length ?? 0, + text: hostDraft?.text ?? "", + attachmentCount: hostDraft?.attachments.length ?? 0, isRunning: false, isSubmitting: false, }); diff --git a/apps/app/src/components/promptbox/NewThreadComposer.tsx b/apps/app/src/components/promptbox/NewThreadComposer.tsx index 58b6005934..fdcc0c9989 100644 --- a/apps/app/src/components/promptbox/NewThreadComposer.tsx +++ b/apps/app/src/components/promptbox/NewThreadComposer.tsx @@ -1013,21 +1013,23 @@ export function NewThreadComposer({ () => promptDraftToInput(currentDraft), [currentDraft], ); + // Identity-stable across keystrokes; the live draft flows through + // getCurrent/subscribeDraft (see PluginComposerHost). const pluginComposerHost = useMemo( () => ({ scope: { kind: "new-thread", projectId }, - draft: currentDraft, textEffectKey: promptDraft.storageKey, getCurrent: promptDraft.getCurrent, + subscribeDraft: promptDraft.subscribe, setDraft: promptDraft.setDraft, focus: () => promptBoxRef.current?.focusEnd(), }), [ - currentDraft, projectId, promptDraft.getCurrent, promptDraft.setDraft, promptDraft.storageKey, + promptDraft.subscribe, ], ); diff --git a/apps/app/src/components/promptbox/PromptBoxActionsMenu.test.tsx b/apps/app/src/components/promptbox/PromptBoxActionsMenu.test.tsx index 7f67b8384f..c9dd5c069d 100644 --- a/apps/app/src/components/promptbox/PromptBoxActionsMenu.test.tsx +++ b/apps/app/src/components/promptbox/PromptBoxActionsMenu.test.tsx @@ -116,9 +116,9 @@ describe("PromptBoxActionsMenu", () => { const setDraft = vi.fn(); const host: PluginComposerHost = { scope: view.scope, - draft, textEffectKey: "plus-menu-update-test", getCurrent: () => draft, + subscribeDraft: () => () => {}, setDraft, focus: () => document.getElementById("composer-focus-target")?.focus(), }; @@ -177,9 +177,9 @@ describe("PromptBoxActionsMenu", () => { const draft = emptyPromptDraftState(); const host: PluginComposerHost = { scope: view.scope, - draft, textEffectKey: "plus-menu-test", getCurrent: () => draft, + subscribeDraft: () => () => {}, setDraft: vi.fn(), focus: vi.fn(), }; diff --git a/apps/app/src/components/promptbox/PromptBoxInternal.test.tsx b/apps/app/src/components/promptbox/PromptBoxInternal.test.tsx index f6e744b883..99a251171d 100644 --- a/apps/app/src/components/promptbox/PromptBoxInternal.test.tsx +++ b/apps/app/src/components/promptbox/PromptBoxInternal.test.tsx @@ -827,9 +827,9 @@ describe("PromptBoxInternal controlled value sync", () => { threadId: "thread-1", queuedMessageId, }, - draft, textEffectKey: `queued-message:${queuedMessageId}`, getCurrent: () => draft, + subscribeDraft: () => () => {}, setDraft: vi.fn(), focus: vi.fn(), }); @@ -1754,9 +1754,9 @@ describe("PromptBoxInternal plugin composer actions", () => { const draft = emptyPromptDraftState(); const host: PluginComposerHost = { scope: { kind: "thread", threadId: "crashing-action-thread" }, - draft, textEffectKey: "crashing-action-composer", getCurrent: () => draft, + subscribeDraft: () => () => {}, setDraft: vi.fn(), focus: vi.fn(), }; @@ -1841,9 +1841,9 @@ describe("PromptBoxInternal plugin composer actions", () => { const draft = emptyPromptDraftState(); const host: PluginComposerHost = { scope: { kind: "thread", threadId: "thread-1" }, - draft, textEffectKey: "promptbox-lock-test", getCurrent: () => draft, + subscribeDraft: () => () => {}, setDraft: vi.fn(), focus: vi.fn(), }; @@ -1930,9 +1930,9 @@ describe("PromptBoxInternal plugin composer actions", () => { const draft = emptyPromptDraftState(); const host = (threadId: string): PluginComposerHost => ({ scope: { kind: "thread", threadId }, - draft, textEffectKey: `scope-action:${threadId}`, getCurrent: () => draft, + subscribeDraft: () => () => {}, setDraft: vi.fn(), focus: vi.fn(), }); diff --git a/apps/app/src/components/thread/embedded-chat/EmbeddedThreadChat.test.tsx b/apps/app/src/components/thread/embedded-chat/EmbeddedThreadChat.test.tsx index 67fbb28b94..9ede19e217 100644 --- a/apps/app/src/components/thread/embedded-chat/EmbeddedThreadChat.test.tsx +++ b/apps/app/src/components/thread/embedded-chat/EmbeddedThreadChat.test.tsx @@ -1,9 +1,11 @@ // @vitest-environment jsdom import { cleanup, fireEvent, render, screen } from "@testing-library/react"; -import type { ReactNode } from "react"; +import { useEffect, useLayoutEffect, type ReactNode } from "react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { FollowUpComposerProps } from "@/components/promptbox/FollowUpPromptBox"; +import type { PluginComposerHost } from "@/components/plugin/plugin-composer-host"; +import { getPromptDraftAccessor } from "@/hooks/usePromptDraftStorage"; import { EmbeddedThreadChat } from "./EmbeddedThreadChat"; const mocks = vi.hoisted(() => ({ @@ -29,31 +31,82 @@ const mocks = vi.hoisted(() => ({ resolveMentionLink: vi.fn(), })); -vi.mock("@/components/promptbox/FollowUpPromptBox", () => ({ - FollowUpPromptBox: ({ - composer, - stack, - }: { - composer: Pick< - FollowUpComposerProps, - "message" | "onChangeMessage" | "onSubmit" - >; - stack: ReactNode; - }) => ( -
- {stack} - composer.onChangeMessage(event.target.value, [])} - /> - -
- ), +const hostDraftMocks = vi.hoisted(() => ({ + /** The bottom host from the most recent FollowUpPromptBox render. */ + latestHost: null as { + getCurrent(): { text: string }; + subscribeDraft(listener: () => void): () => void; + } | null, + /** latestHost.getCurrent().text captured inside each subscribeDraft notification. */ + textAtNotify: [] as string[], + subscribed: false, })); +vi.mock("@/components/promptbox/FollowUpPromptBox", async () => { + const { usePluginComposerHostDraft } = await import( + "@/components/plugin/plugin-composer-host" + ); + // A host-draft subscriber, like plugin surfaces reading useComposerView(). + function BottomHostDraftProbe({ + host, + }: { + host: PluginComposerHost | null; + }) { + // Record what the CURRENT host's getCurrent() returns at the exact moment + // subscribeDraft notifies. useSyncExternalStore reads the snapshot inside + // the notification to decide whether to re-render, so a notify that fires + // before the host's identity refs are synced hands every subscriber a + // stale draft with no later correction. The subscription is established + // once and deliberately outlives probe remounts (the notifier function is + // shared by successive hosts of one EmbeddedThreadChat instance) so the + // recorder observes the notification even when the composer subtree is + // remounted by the switch. The latest-host sync is a layout effect: the + // probe is a child of EmbeddedThreadChat, so it runs before the parent's + // notifier effect fires. + useLayoutEffect(() => { + hostDraftMocks.latestHost = host; + }, [host]); + useEffect(() => { + if (hostDraftMocks.subscribed || !host) return; + hostDraftMocks.subscribed = true; + host.subscribeDraft(() => { + hostDraftMocks.textAtNotify.push( + hostDraftMocks.latestHost?.getCurrent().text ?? "", + ); + }); + }, [host]); + const draft = usePluginComposerHostDraft(host); + return
{draft?.text ?? ""}
; + } + return { + FollowUpPromptBox: ({ + composer, + stack, + pluginComposerHost, + }: { + composer: Pick< + FollowUpComposerProps, + "message" | "onChangeMessage" | "onSubmit" + >; + stack: ReactNode; + pluginComposerHost?: PluginComposerHost | null; + }) => ( +
+ {stack} + composer.onChangeMessage(event.target.value, [])} + /> + + +
+ ), + }; +}); + vi.mock("@/components/promptbox/banner/QueuedMessagesList", () => ({ QueuedMessagesList: ({ queuedMessages, @@ -272,14 +325,16 @@ vi.mock("@/hooks/mutations/project-mutations", () => ({ }), })); -function renderEmbeddedChat({ +function buildEmbeddedChat({ threadId = "thr_child", surfaceTone = "background", + pluginComposerBottomScope, }: { threadId?: string; surfaceTone?: "background" | "sidebar"; + pluginComposerBottomScope?: PluginComposerHost["scope"]; } = {}) { - return render( + return ( , + /> ); } +function renderEmbeddedChat( + options: Parameters[0] = {}, +) { + return render(buildEmbeddedChat(options)); +} + describe("EmbeddedThreadChat", () => { beforeEach(() => { window.localStorage.clear(); @@ -323,6 +385,9 @@ describe("EmbeddedThreadChat", () => { mocks.timelinePanelProps = []; mocks.timelineProjectIds = []; mocks.resolveMentionLink.mockReset(); + hostDraftMocks.latestHost = null; + hostDraftMocks.textAtNotify = []; + hostDraftMocks.subscribed = false; }); it("applies the requested surface tone to the timeline and footer", () => { @@ -488,4 +553,63 @@ describe("EmbeddedThreadChat", () => { expect(screen.queryByTestId("pending-interaction-banner")).toBeNull(); expect(screen.getByTestId("embedded-chat-composer")).toBeTruthy(); }); + + // The bottom host's getCurrent gates on the active-identity ref, and that + // ref is synced in a layout effect. A thread switch changes the host + // identity and the draft in ONE commit: if the draft notifier's effect ran + // before the identity sync, subscribers were notified while getCurrent + // still resolved to the pre-switch fallback draft — and no later effect + // notified again, so plugin surfaces showed the previous thread's draft + // until the next keystroke. + it("delivers the new thread's draft to host subscribers immediately on a thread switch", () => { + getPromptDraftAccessor({ + kind: "thread", + projectId: "proj-1", + threadId: "thr_switch_a", + }).setDraft({ text: "alpha draft", mentions: [], attachments: [] }); + getPromptDraftAccessor({ + kind: "thread", + projectId: "proj-1", + threadId: "thr_switch_b", + }).setDraft({ text: "beta draft", mentions: [], attachments: [] }); + + const scopeFor = (threadId: string) => + ({ kind: "thread", threadId }) as const; + const view = render( + buildEmbeddedChat({ + threadId: "thr_switch_a", + pluginComposerBottomScope: scopeFor("thr_switch_a"), + }), + ); + expect(screen.getByTestId("embedded-host-draft").textContent).toBe( + "alpha draft", + ); + + // Identity + draft change in the same commit. + view.rerender( + buildEmbeddedChat({ + threadId: "thr_switch_b", + pluginComposerBottomScope: scopeFor("thr_switch_b"), + }), + ); + expect(screen.getByTestId("embedded-host-draft").textContent).toBe( + "beta draft", + ); + // The switch's notification must already observe the new draft: this is + // the read useSyncExternalStore performs inside the notify, and there is + // no later notification to correct a stale one. + expect(hostDraftMocks.textAtNotify).toEqual(["beta draft"]); + + // And back, with no intervening keystroke. + view.rerender( + buildEmbeddedChat({ + threadId: "thr_switch_a", + pluginComposerBottomScope: scopeFor("thr_switch_a"), + }), + ); + expect(screen.getByTestId("embedded-host-draft").textContent).toBe( + "alpha draft", + ); + expect(hostDraftMocks.textAtNotify).toEqual(["beta draft", "alpha draft"]); + }); }); diff --git a/apps/app/src/components/thread/embedded-chat/EmbeddedThreadChat.tsx b/apps/app/src/components/thread/embedded-chat/EmbeddedThreadChat.tsx index f947b65981..00f1208948 100644 --- a/apps/app/src/components/thread/embedded-chat/EmbeddedThreadChat.tsx +++ b/apps/app/src/components/thread/embedded-chat/EmbeddedThreadChat.tsx @@ -20,7 +20,10 @@ import { FollowUpPromptBox, type FollowUpComposerProps, } from "@/components/promptbox/FollowUpPromptBox"; -import type { PluginComposerHost } from "@/components/plugin/plugin-composer-host"; +import { + useComposerHostDraftNotifier, + type PluginComposerHost, +} from "@/components/plugin/plugin-composer-host"; import { ThreadPendingInteractionBanner } from "@/components/thread/pending-interactions/ThreadPendingInteractionBanner"; import { QueuedMessagesList, @@ -667,6 +670,16 @@ function EmbeddedThreadChatWithComposer({ } }; }, [queuedComposerHostIdentity]); + // subscribeDraft sources for the two hosts below. The notifiers MUST be + // declared after every ref-sync layout effect above (hook order = effect + // order): a thread switch changes the host identity and the draft in one + // commit, and `getCurrent` gates on the active-identity refs. Notifying + // before the identity sync would hand subscribers the stale pre-switch + // draft with no later notification to correct it. + const subscribeBottomDraft = useComposerHostDraftNotifier(currentPromptDraft); + const subscribeQueuedDraft = useComposerHostDraftNotifier( + inlineEditingQueuedMessage?.draft ?? null, + ); const setStoredPromptDraft = promptDraft.setDraft; const bottomPluginComposerHost = useMemo(() => { if (bottomScope === null) return null; @@ -675,11 +688,11 @@ function EmbeddedThreadChatWithComposer({ return { scope: bottomScope, textEffectKey: identity, - draft: currentPromptDraftRef.current, getCurrent: () => activeBottomComposerIdentityRef.current === identity ? currentPromptDraftRef.current : initialDraft, + subscribeDraft: subscribeBottomDraft, setDraft: (draft) => { if (activeBottomComposerIdentityRef.current === identity) { setStoredPromptDraft(draft); @@ -691,7 +704,12 @@ function EmbeddedThreadChatWithComposer({ } }, }; - }, [bottomComposerHostIdentity, bottomScope, setStoredPromptDraft]); + }, [ + bottomComposerHostIdentity, + bottomScope, + setStoredPromptDraft, + subscribeBottomDraft, + ]); const queuedPluginComposerHost = useMemo(() => { if ( queuedComposerIdentity === null || @@ -720,7 +738,6 @@ function EmbeddedThreadChatWithComposer({ queuedMessageId: queuedEdit.queuedMessageId, }, textEffectKey: identity, - draft: initialDraft, getCurrent: () => { if (activeQueuedComposerIdentityRef.current !== identity) { return initialDraft; @@ -730,6 +747,7 @@ function EmbeddedThreadChatWithComposer({ ? currentQueuedEdit.draft : initialDraft; }, + subscribeDraft: subscribeQueuedDraft, setDraft: (draft) => { if (activeQueuedComposerIdentityRef.current !== identity) { return; @@ -748,24 +766,11 @@ function EmbeddedThreadChatWithComposer({ inlineEditingQueuedMessageRef, queuedComposerIdentity, queuedComposerHostIdentity, + subscribeQueuedDraft, updateInlineQueuedMessage, ]); - const bottomPluginComposerHostWithDraft = useMemo( - () => - bottomPluginComposerHost === null - ? null - : { ...bottomPluginComposerHost, draft: currentPromptDraft }, - [bottomPluginComposerHost, currentPromptDraft], - ); - const queuedPluginComposerHostWithDraft = useMemo( - () => - queuedPluginComposerHost === null - ? null - : { ...queuedPluginComposerHost, draft: activeComposerDraft }, - [activeComposerDraft, queuedPluginComposerHost], - ); - const activeBottomPluginComposerHost = bottomPluginComposerHostWithDraft; - const activeQueuedPluginComposerHost = queuedPluginComposerHostWithDraft; + const activeBottomPluginComposerHost = bottomPluginComposerHost; + const activeQueuedPluginComposerHost = queuedPluginComposerHost; const bottomComposerTextEffects = useComposerTextEffects( activeBottomPluginComposerHost?.textEffectKey ?? null, ); diff --git a/apps/app/src/hooks/usePromptDraftStorage.ts b/apps/app/src/hooks/usePromptDraftStorage.ts index 086f247068..3b35280371 100644 --- a/apps/app/src/hooks/usePromptDraftStorage.ts +++ b/apps/app/src/hooks/usePromptDraftStorage.ts @@ -289,6 +289,7 @@ function getPromptDraftStorageKey(scope: PromptDraftScope): string { export function getPromptDraftAccessor(scope: PromptDraftScope): { storageKey: string; getCurrent: () => PromptDraftState; + subscribe: (listener: () => void) => () => void; setDraft: (draft: PromptDraftState) => void; addQuote: ( text: string, @@ -299,6 +300,7 @@ export function getPromptDraftAccessor(scope: PromptDraftScope): { return { storageKey, getCurrent: () => readPromptDraft(storageKey), + subscribe: (listener) => subscribePromptDraft(storageKey, listener), setDraft: (draft) => writePromptDraft(storageKey, draft), addQuote: (text, attachments) => addQuoteToPromptDraft(storageKey, text, attachments), @@ -327,6 +329,13 @@ export function usePromptDraftStorage(scope: PromptDraftScope) { return readPromptDraft(storageKey); }, [storageKey]); + // Stable per storage key, so a plugin composer host built over this draft + // can expose the store subscription without re-creating the host per write. + const subscribe = useCallback( + (listener: () => void) => subscribePromptDraft(storageKey, listener), + [storageKey], + ); + const setTextAndMentions = useCallback( (nextText: string, nextMentions: PromptTextMention[]) => { writePromptDraft( @@ -421,6 +430,7 @@ export function usePromptDraftStorage(scope: PromptDraftScope) { () => ({ storageKey, getCurrent, + subscribe, value: draft.text, text: draft.text, mentions: draft.mentions, @@ -450,6 +460,7 @@ export function usePromptDraftStorage(scope: PromptDraftScope) { setDraftAndPersist, setTextAndMentions, storageKey, + subscribe, ], ); } diff --git a/apps/app/src/lib/plugin-sdk-hooks.ts b/apps/app/src/lib/plugin-sdk-hooks.ts index 2737dc3efd..a9cc98cd1b 100644 --- a/apps/app/src/lib/plugin-sdk-hooks.ts +++ b/apps/app/src/lib/plugin-sdk-hooks.ts @@ -34,6 +34,7 @@ import { usePluginThreadPanelOpenHandler } from "@/components/plugin/plugin-thre import { PluginComposerViewContext, usePluginComposerHost, + usePluginComposerHostDraft, } from "@/components/plugin/plugin-composer-host"; import { sdk } from "@/lib/sdk"; import { useSystemProviders } from "@/hooks/queries/system-queries"; @@ -622,7 +623,8 @@ export function useComposerView(): ComposerView { [projectId, threadId], ); const routeDraft = usePromptDraftStorage(routeScope); - const draft = composerHost?.draft ?? routeDraft; + const hostDraft = usePluginComposerHostDraft(composerHost); + const draft = hostDraft ?? routeDraft; const fallback = useMemo( () => ({ scope: @@ -656,6 +658,7 @@ export function useComposer(): PluginComposerApi { const pluginId = usePluginId(); const slotOwnershipRegistry = useContext(PluginSlotOwnershipContext); const composerHost = usePluginComposerHost(); + const composerHostDraft = usePluginComposerHostDraft(composerHost); const { projectId, threadId } = useRouteState(); const routeScope: PromptDraftScope = useMemo( () => @@ -852,7 +855,7 @@ export function useComposer(): PluginComposerApi { ); const focus = focusActiveComposer; - const composerText = composerHost?.draft.text ?? routeDraft.text; + const composerText = composerHostDraft?.text ?? routeDraft.text; return useMemo( () => ({ diff --git a/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx b/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx index 1fb45b2765..96314a8f48 100644 --- a/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx +++ b/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx @@ -283,9 +283,9 @@ vi.mock("./ThreadDetailView", () => ({ const draft = { attachments: [], mentions: [], text: "" }; return { scope: { kind: "thread", threadId }, - draft, textEffectKey: `test-draft-${threadId}`, getCurrent: () => draft, + subscribeDraft: () => () => {}, setDraft: () => undefined, focus: () => undefined, }; diff --git a/apps/app/src/views/thread-detail/ThreadDetailPromptArea.keystrokes.test.tsx b/apps/app/src/views/thread-detail/ThreadDetailPromptArea.keystrokes.test.tsx new file mode 100644 index 0000000000..029d2c1f99 --- /dev/null +++ b/apps/app/src/views/thread-detail/ThreadDetailPromptArea.keystrokes.test.tsx @@ -0,0 +1,582 @@ +// @vitest-environment jsdom + +import type { + PendingInteraction, + ThreadQueuedMessage, + ThreadWithRuntime, +} from "@bb/domain"; +import { + act, + cleanup, + fireEvent, + render, + screen, + within, +} from "@testing-library/react"; +import type { ReactNode } from "react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + PluginComposerHostScopeProvider, + usePluginComposerHost, + usePluginComposerHostDraft, +} from "@/components/plugin/plugin-composer-host"; +import { getPromptDraftAccessor } from "@/hooks/usePromptDraftStorage"; +import { ThreadDetailPromptArea } from "./ThreadDetailPromptArea"; + +/** + * Keystroke isolation for the published plugin composer host. + * + * The host published by ThreadDetailPromptArea reaches large non-draft + * subscribers (ThreadDetailSecondaryContentBody -> SecondaryPanelLayout -> + * ThreadTimelinePane, the hosted-panel registry). It must stay referentially + * stable while the user types: a per-keystroke identity notified the pane + * scope and re-rendered the whole thread shell per character. The live draft + * must instead reach actual draft consumers through the host's + * getCurrent/subscribeDraft pair. These tests use the real draft store so + * keystrokes flow the way they do in the app. + */ + +const mocks = vi.hoisted(() => ({ + sendMessageMutateAsync: vi.fn(), + shellProbeRenders: vi.fn(), + updateQueuedMessageMutateAsync: vi.fn(), +})); + +vi.mock("react-router-dom", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useNavigate: () => vi.fn() }; +}); + +vi.mock("@/components/promptbox/FollowUpPromptBox", () => ({ + FollowUpPromptBox: ({ + composer, + pendingInteraction = null, + stack, + }: { + composer: { + message: string; + onChangeMessage: (message: string, mentions: []) => void; + onSubmit: () => void; + } | null; + pendingInteraction?: ReactNode; + stack: ReactNode; + }) => ( +
+
+ {stack} + {pendingInteraction} +
+ {composer ? ( + // Like the real FollowUpPromptBox: hidden, not unmounted, while a + // pending interaction takes the composer's place. + + ) : null} +
+ ), +})); + +vi.mock("@/components/promptbox/ThreadEnvironmentSummary", () => ({ + ThreadEnvironmentSummary: () =>
, +})); + +vi.mock("@/components/promptbox/banner/QueuedMessagesList", () => ({ + QueuedMessagesList: ({ + inlineEditor, + queuedMessages, + onEdit, + }: { + inlineEditor?: { content: ReactNode; onDismiss: () => void }; + queuedMessages: readonly ThreadQueuedMessage[]; + onEdit: (request: { + queuedMessageId: string; + queuedMessageIndex: number; + }) => void; + }) => ( +
+ {queuedMessages.map((message, index) => ( + + ))} + {inlineEditor ? ( +
+ {inlineEditor.content} + +
+ ) : null} +
+ ), +})); + +vi.mock("@/components/promptbox/banner/ThreadBackgroundCommandsCard", () => ({ + ThreadBackgroundCommandsCard: () => null, +})); + +vi.mock("@/components/promptbox/banner/ThreadGoalCard", () => ({ + ThreadGoalCard: () => null, +})); + +vi.mock("@/components/promptbox/banner/ThreadPromptContextBanner", () => ({ + ThreadPromptContextBanner: () => null, +})); + +vi.mock("@/components/promptbox/banner/ThreadPromptModeCard", () => ({ + ThreadPromptModeCard: () => null, +})); + +vi.mock("@/components/promptbox/banner/ThreadTodoCard", () => ({ + ThreadTodoCard: () => null, +})); + +vi.mock("@/components/promptbox/banner/ThreadWorkflowCard", () => ({ + ThreadWorkflowCard: () => null, +})); + +vi.mock( + "@/components/thread/pending-interactions/ThreadPendingInteractionBanner", + () => ({ + ThreadPendingInteractionBanner: () => ( +
+ ), + }), +); + +vi.mock("@/components/plugin/PluginPendingInteractionComposer", () => ({ + PluginPendingInteractionComposer: () => null, +})); + +vi.mock("@/components/ui/app-toast", () => ({ + appToast: { error: vi.fn() }, +})); + +vi.mock("@/hooks/useCommandSuggestions", () => ({ + useCommandSuggestions: () => ({ + hasMore: false, + isError: false, + isLoading: false, + isLoadingMore: false, + loadMore: vi.fn(), + suggestions: [], + trigger: null, + }), +})); + +vi.mock("@/hooks/usePromptMentions", () => ({ + usePromptMentions: () => ({ + isError: false, + isLoading: false, + setQuery: vi.fn(), + suggestions: [], + }), +})); + +vi.mock("@/hooks/useThreadCreationOptions", () => ({ + useThreadCreationOptions: () => ({ + activeModel: null, + executionInputSources: {}, + hasMultipleProviders: false, + isLoadingModels: false, + modelLoadError: null, + modelLoadFailed: false, + modelOptions: [], + moreModelOptions: [], + permissionMode: "auto", + permissionModeOptions: [], + providerOptions: [], + reasoningLevel: "medium", + reasoningOptions: [], + selectedModel: "gpt-5", + selectedProviderComposerActions: [], + selectedProviderDisplayName: "Codex", + selectedProviderId: "codex", + serviceTier: undefined, + serviceTierSupportByProvider: {}, + setPermissionMode: vi.fn(), + setReasoningLevel: vi.fn(), + setSelectedModel: vi.fn(), + setServiceTier: vi.fn(), + supportsPermissionModeSelection: true, + supportsServiceTier: false, + }), +})); + +vi.mock("@/hooks/mutations/project-mutations", () => ({ + useUploadPromptAttachment: () => ({ + isPending: false, + mutateAsync: vi.fn(), + }), +})); + +vi.mock("@/hooks/mutations/thread-runtime-mutations", () => { + const idleMutation = () => ({ + isPending: false, + mutate: vi.fn(), + mutateAsync: vi.fn(), + variables: null, + }); + return { + useCancelThreadPlan: idleMutation, + useClearThreadGoal: idleMutation, + useCreateThreadQueuedMessage: idleMutation, + useDeleteThreadQueuedMessage: idleMutation, + useReorderThreadQueuedMessage: idleMutation, + useSetThreadQueuedMessageGroupBoundary: idleMutation, + useSendThreadQueuedMessage: idleMutation, + useStopThread: idleMutation, + useUpdateThreadQueuedMessage: () => ({ + isPending: false, + mutateAsync: mocks.updateQueuedMessageMutateAsync, + }), + }; +}); + +vi.mock("@/hooks/mutations/thread-state-mutations", () => ({ + useUnarchiveThread: () => ({ + isPending: false, + mutate: vi.fn(), + variables: null, + }), +})); + +vi.mock("@/hooks/queries/sidebar-navigation-query", () => ({ + useProjectDisplayName: () => null, +})); + +vi.mock("@/hooks/queries/thread-default-execution-options-query", () => ({ + useThreadDefaultExecutionOptions: () => ({ + data: { + model: "gpt-5", + permissionMode: "auto", + reasoningLevel: "medium", + serviceTier: "default", + source: "client/turn/requested", + }, + isError: false, + }), +})); + +const queryMocks = vi.hoisted(() => ({ + queuedMessages: [] as ThreadQueuedMessage[], +})); + +vi.mock("@/hooks/queries/thread-queries", () => ({ + getLatestPendingInteraction: (interactions: readonly PendingInteraction[]) => + interactions.at(-1) ?? null, + useThreadPromptHistory: () => ({ data: [] }), + useThreadQueuedMessages: () => ({ data: queryMocks.queuedMessages }), +})); + +const PROJECT_ID = "proj_keystrokes"; + +function makeThread(id: string): ThreadWithRuntime { + return { + archivedAt: null, + environmentId: null, + id, + projectId: PROJECT_ID, + providerId: "codex", + runtime: { displayStatus: "idle" }, + status: "idle", + } as ThreadWithRuntime; +} + +function makeQueuedMessage(): ThreadQueuedMessage { + return { + id: "qmsg_1", + content: [{ type: "text", text: "Already queued", mentions: [] }], + model: "gpt-5", + reasoningLevel: "medium", + permissionMode: "auto", + serviceTier: "default", + groupWithNext: false, + createdAt: 1, + updatedAt: 1, + }; +} + +function makePendingInteraction(threadId: string): PendingInteraction { + return { + id: `interaction-${threadId}`, + threadId, + turnId: "turn-1", + providerId: "codex", + providerThreadId: "provider-thread-1", + providerRequestId: "provider-request-1", + origin: { + kind: "provider", + providerId: "codex", + providerThreadId: "provider-thread-1", + providerRequestId: "provider-request-1", + }, + payload: { + kind: "user_question", + questions: [ + { + id: "question-1", + prompt: "Continue?", + multiSelect: false, + allowFreeText: true, + }, + ], + }, + resolution: null, + status: "pending", + statusReason: null, + createdAt: 1, + resolvedAt: null, + }; +} + +/** + * Mirrors ThreadDetailSecondaryContentBody: holds the published host without + * reading its draft. Every render is one shell re-render in the app + * (SecondaryPanelLayout, ThreadTimelinePane), so this must stay flat while + * the user types. + */ +function ShellProbe() { + mocks.shellProbeRenders(usePluginComposerHost()); + return null; +} + +/** An actual draft consumer (the plugin-hook read path). */ +function PublishedHostDraftProbe() { + const host = usePluginComposerHost(); + const draft = usePluginComposerHostDraft(host); + return
{draft?.text ?? ""}
; +} + +function observedShellHosts(): readonly unknown[] { + return mocks.shellProbeRenders.mock.calls.map((call) => call[0]); +} + +/** + * Mounting settles at two shell renders: the probe first sees an empty scope, + * then the area's layout-effect publish delivers the host. Everything after + * that baseline is a real shell re-render. + */ +function shellRenderCount(): number { + return mocks.shellProbeRenders.mock.calls.length; +} + +interface RenderPromptAreaArgs { + thread: ThreadWithRuntime; + pendingInteractions?: readonly PendingInteraction[]; +} + +function buildPromptArea({ + thread, + pendingInteractions = [], +}: RenderPromptAreaArgs) { + return ( + + + + null} + sendMessage={{ + isPending: false, + mutateAsync: mocks.sendMessageMutateAsync, + }} + steerActiveThreadOnEnter={false} + thread={thread} + workspaceChangedFilesSection={null} + workspaceStatusPending={false} + /> + + ); +} + +function renderPromptArea(args: RenderPromptAreaArgs) { + return render(buildPromptArea(args)); +} + +function getBottomComposerInput(): HTMLInputElement { + return screen.getByRole("textbox", { + name: "Composer message", + }) as HTMLInputElement; +} + +let threadCounter = 0; +let threadId = ""; + +beforeEach(() => { + threadCounter += 1; + threadId = `thr_keystrokes_${threadCounter}`; + queryMocks.queuedMessages = []; + mocks.sendMessageMutateAsync.mockResolvedValue(undefined); + mocks.updateQueuedMessageMutateAsync.mockResolvedValue(undefined); +}); + +afterEach(() => { + cleanup(); + window.localStorage.clear(); + vi.clearAllMocks(); +}); + +describe("ThreadDetailPromptArea published composer host", () => { + it("keeps the published host referentially stable while keystrokes reach draft consumers", () => { + renderPromptArea({ thread: makeThread(threadId) }); + const input = getBottomComposerInput(); + const rendersAfterMount = shellRenderCount(); + const hostAfterMount = observedShellHosts().at(-1); + expect(hostAfterMount).not.toBe(null); + + const typed = "abcdefghijklmnopqrstu"; + for (let index = 1; index <= typed.length; index += 1) { + fireEvent.change(input, { target: { value: typed.slice(0, index) } }); + } + + expect(input.value).toBe(typed); + // Draft consumers saw every keystroke through the stable host... + expect(screen.getByTestId("published-host-draft").textContent).toBe(typed); + // ...while the pane scope never notified: no shell re-render for the + // entire burst, including the empty -> non-empty flip. + expect(shellRenderCount()).toBe(rendersAfterMount); + expect(observedShellHosts().at(-1)).toBe(hostAfterMount); + }); + + it("submits the draft as typed, read imperatively at event time", async () => { + renderPromptArea({ thread: makeThread(threadId) }); + const input = getBottomComposerInput(); + for (const index of Array.from({ length: 7 }, (_, i) => i + 1)) { + fireEvent.change(input, { target: { value: "Ship it".slice(0, index) } }); + } + + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "Submit composer" })); + }); + + expect(mocks.sendMessageMutateAsync).toHaveBeenCalledTimes(1); + expect(mocks.sendMessageMutateAsync.mock.calls[0]?.[0]).toMatchObject({ + input: [{ type: "text", text: "Ship it", mentions: [] }], + }); + expect( + getPromptDraftAccessor({ + kind: "thread", + projectId: PROJECT_ID, + threadId, + }).getCurrent().text, + ).toBe(""); + }); + + it("delivers external draft writes to consumers without re-rendering the shell, even while a pending interaction hides the composer", () => { + const accessor = getPromptDraftAccessor({ + kind: "thread", + projectId: PROJECT_ID, + threadId, + }); + renderPromptArea({ + thread: makeThread(threadId), + pendingInteractions: [makePendingInteraction(threadId)], + }); + expect(screen.getByTestId("pending-interaction")).toBeTruthy(); + expect(screen.getByTestId("published-host-draft").textContent).toBe(""); + const rendersAfterMount = shellRenderCount(); + + act(() => { + accessor.setDraft({ + text: "typed elsewhere", + mentions: [], + attachments: [], + }); + }); + expect(screen.getByTestId("published-host-draft").textContent).toBe( + "typed elsewhere", + ); + + act(() => { + accessor.setDraft({ + text: "typed elsewhere again", + mentions: [], + attachments: [], + }); + }); + expect(screen.getByTestId("published-host-draft").textContent).toBe( + "typed elsewhere again", + ); + expect(shellRenderCount()).toBe(rendersAfterMount); + }); + + it("swaps to a per-session stable host for inline queued-message edits and streams the inline draft", () => { + queryMocks.queuedMessages = [makeQueuedMessage()]; + renderPromptArea({ thread: makeThread(threadId) }); + const rendersAfterMount = shellRenderCount(); + const threadHost = observedShellHosts().at(-1); + + fireEvent.click( + screen.getByRole("button", { name: "Edit queued message 1" }), + ); + // Opening the editor publishes the queued-message host: exactly one + // legitimate shell notification. + expect(shellRenderCount()).toBe(rendersAfterMount + 1); + expect(observedShellHosts().at(-1)).not.toBe(threadHost); + expect(screen.getByTestId("published-host-draft").textContent).toBe( + "Already queued", + ); + + const inlineInput = within( + screen.getByTestId("inline-queued-message-editor"), + ).getByRole("textbox", { name: "Composer message" }) as HTMLInputElement; + const typed = "Already queued and refined"; + for ( + let index = "Already queued".length + 1; + index <= typed.length; + index += 1 + ) { + fireEvent.change(inlineInput, { + target: { value: typed.slice(0, index) }, + }); + } + + expect(inlineInput.value).toBe(typed); + // Inline keystrokes reached consumers through the same stable host... + expect(screen.getByTestId("published-host-draft").textContent).toBe(typed); + expect(shellRenderCount()).toBe(rendersAfterMount + 1); + + // ...and closing the editor swaps back to the identical thread host. + fireEvent.click(screen.getByRole("button", { name: "Cancel queued edit" })); + expect(shellRenderCount()).toBe(rendersAfterMount + 2); + expect(observedShellHosts().at(-1)).toBe(threadHost); + expect(screen.getByTestId("published-host-draft").textContent).toBe(""); + }); +}); diff --git a/apps/app/src/views/thread-detail/ThreadDetailPromptArea.test.tsx b/apps/app/src/views/thread-detail/ThreadDetailPromptArea.test.tsx index 5589926d05..553cf786be 100644 --- a/apps/app/src/views/thread-detail/ThreadDetailPromptArea.test.tsx +++ b/apps/app/src/views/thread-detail/ThreadDetailPromptArea.test.tsx @@ -56,6 +56,7 @@ const mocks = vi.hoisted(() => ({ setDraft: vi.fn(), setTextAndMentions: vi.fn(), storageKey: "bb.promptbox.contents-proj_1-thr_1-3", + subscribe: vi.fn(() => () => {}), text: "", }, queuedMessages: [] as ThreadQueuedMessage[], @@ -206,7 +207,7 @@ vi.mock("@/components/promptbox/FollowUpPromptBox", async () => { type="button" onClick={() => pluginComposerHost.setDraft({ - ...pluginComposerHost.draft, + ...pluginComposerHost.getCurrent(), text: "Plugin-enhanced queued message", }) } diff --git a/apps/app/src/views/thread-detail/ThreadDetailPromptArea.tsx b/apps/app/src/views/thread-detail/ThreadDetailPromptArea.tsx index fe46495a31..6edeed86ca 100644 --- a/apps/app/src/views/thread-detail/ThreadDetailPromptArea.tsx +++ b/apps/app/src/views/thread-detail/ThreadDetailPromptArea.tsx @@ -37,6 +37,7 @@ import { ThreadPendingInteractionBanner } from "@/components/thread/pending-inte import { PluginPendingInteractionComposer } from "@/components/plugin/PluginPendingInteractionComposer"; import { type PluginComposerHost, + useComposerHostDraftNotifier, usePublishPluginComposerHost, } from "@/components/plugin/plugin-composer-host"; import { @@ -98,6 +99,7 @@ import { getThreadDisplayTitle } from "@/lib/thread-title"; import { buildThreadHandoffLocationState } from "@bb/client-core"; import { appToast } from "@/components/ui/app-toast"; import { + emptyPromptDraftState, promptDraftToInput, type PromptDraftAttachment, type PromptDraftState, @@ -328,6 +330,12 @@ function isInlineQueuedMessageEditSession( ); } +/** + * Fallback for host draft reads that outlive their editor session: the ref no + * longer holds the session, so there is no live draft to return. + */ +const ENDED_EDIT_SESSION_DRAFT = emptyPromptDraftState(); + /** Plugin composer-host accessors for the queued-message inline editor (see below). */ function readInlineQueuedMessageDraft( editStateRef: RefObject, @@ -529,6 +537,14 @@ export function ThreadDetailPromptArea({ inlineEditingQueuedMessageRef, commitInlineQueuedMessage, }); + // subscribeDraft sources for the two state-backed editor hosts. The bottom + // composer's host subscribes through the prompt-draft store directly. + const subscribeInlineQueuedDraft = useComposerHostDraftNotifier( + inlineEditingQueuedMessage?.draft ?? null, + ); + const subscribeSentMessageEditDraft = useComposerHostDraftNotifier( + sentMessageEdit?.draft ?? null, + ); const updateSentMessageEditDraft = sentMessageEdit?.updateDraft; const addSentMessageEditAttachment = useCallback( (attachment: PromptDraftAttachment) => { @@ -791,13 +807,16 @@ export function ThreadDetailPromptArea({ const compactPromptPlaceholder = isStopRequested ? "Stopping thread..." : getCompactFollowUpPromptPlaceholder(runtimeDisplayStatus); - const normalPluginComposerHostBinding = useMemo< - Omit - >( + // Identity-stable across keystrokes: the published host is held by large + // non-draft subscribers (the secondary-content body, the hosted-panel + // registry), so a per-keystroke host identity re-rendered the whole thread + // shell per character. The live draft flows through getCurrent/subscribeDraft. + const normalPluginComposerHost = useMemo( () => ({ scope: { kind: "thread", threadId: thread.id }, textEffectKey: promptDraft.storageKey, getCurrent: promptDraft.getCurrent, + subscribeDraft: promptDraft.subscribe, setDraft: promptDraft.setDraft, focus: focusBottomPluginComposer, }), @@ -806,16 +825,10 @@ export function ThreadDetailPromptArea({ promptDraft.getCurrent, promptDraft.setDraft, promptDraft.storageKey, + promptDraft.subscribe, thread.id, ], ); - const normalPluginComposerHost = useMemo( - () => ({ - ...normalPluginComposerHostBinding, - draft: currentPromptDraft, - }), - [currentPromptDraft, normalPluginComposerHostBinding], - ); const hasPromptDraftInput = currentPromptDraftInput.length > 0; const canSubmitModifierShortcut = canSubmitFollowUpShortcut({ hasPromptDraftInput, @@ -1291,43 +1304,63 @@ export function ThreadDetailPromptArea({ ), [clearThreadGoal.isPending, goal, handleClearGoal, isGoalExpanded], ); + // Stable for the whole edit session (keyed on the session scalars, not the + // per-keystroke edit state), so publishing it does not churn the pane scope + // while the user types in the inline editor. + const inlineEditSessionId = inlineEditingQueuedMessage?.editSessionId ?? null; + const inlineEditQueuedMessageId = + inlineEditingQueuedMessage?.queuedMessageId ?? null; + const queuedMessagePluginComposerHost = + useMemo(() => { + if (inlineEditSessionId === null || inlineEditQueuedMessageId === null) { + return null; + } + const session = { + editSessionId: inlineEditSessionId, + queuedMessageId: inlineEditQueuedMessageId, + }; + return { + scope: { + kind: "queued-message", + threadId: thread.id, + queuedMessageId: inlineEditQueuedMessageId, + }, + textEffectKey: `queued-message:${thread.id}:${inlineEditQueuedMessageId}:${inlineEditSessionId}`, + getCurrent: () => + readInlineQueuedMessageDraft( + inlineEditingQueuedMessageRef, + session, + ENDED_EDIT_SESSION_DRAFT, + ), + subscribeDraft: subscribeInlineQueuedDraft, + setDraft: (draft) => + writeInlineQueuedMessageDraft( + inlineEditingQueuedMessageRef, + session, + draft, + commitInlineQueuedMessage, + ), + focus: focusInlinePluginComposer, + }; + }, [ + commitInlineQueuedMessage, + focusInlinePluginComposer, + inlineEditQueuedMessageId, + inlineEditSessionId, + inlineEditingQueuedMessageRef, + subscribeInlineQueuedDraft, + thread.id, + ]); const queuedMessageEditor = useMemo(() => { if ( !inlineEditingQueuedMessage || !inlineExecutionConfig || - !inlinePermissionConfig + !inlinePermissionConfig || + !queuedMessagePluginComposerHost ) { return null; } - const { - draft: initialDraft, - editSessionId, - queuedMessageId, - } = inlineEditingQueuedMessage; - const session = { editSessionId, queuedMessageId }; - const pluginComposerHost: PluginComposerHost = { - scope: { - kind: "queued-message", - threadId: thread.id, - queuedMessageId, - }, - textEffectKey: `queued-message:${thread.id}:${queuedMessageId}:${editSessionId}`, - draft: activeComposerDraft, - getCurrent: () => - readInlineQueuedMessageDraft( - inlineEditingQueuedMessageRef, - session, - initialDraft, - ), - setDraft: (draft) => - writeInlineQueuedMessageDraft( - inlineEditingQueuedMessageRef, - session, - draft, - commitInlineQueuedMessage, - ), - focus: focusInlinePluginComposer, - }; + const { editSessionId, queuedMessageId } = inlineEditingQueuedMessage; const inlineEditor: QueuedMessageInlineEditor = { queuedMessageId, queuedMessageIndex: inlineEditingQueuedMessage.queuedMessageIndex, @@ -1354,7 +1387,7 @@ export function ThreadDetailPromptArea({ onChangeMessage: handleComposerMessageChange, onSelectHistoryEntry: setActiveComposerDraft, permission: inlinePermissionConfig, - pluginComposerHost, + pluginComposerHost: queuedMessagePluginComposerHost, promptActions, promptPlaceholder, submit: handleInlineComposerSubmit, @@ -1365,21 +1398,18 @@ export function ThreadDetailPromptArea({ collapseResetKey: `queued-message:${queuedMessageId}`, }), }; - return { inlineEditor, pluginComposerHost }; + return inlineEditor; }, [ activeComposerDraft, activeComposerDraftInput.length, - commitInlineQueuedMessage, compactPromptPlaceholder, dismissInlineQueuedMessageEditor, editFocusNonce, - focusInlinePluginComposer, handleAttachInlineFiles, handleComposerMessageChange, handleInlineComposerSubmit, inlineAttachmentError, inlineEditingQueuedMessage, - inlineEditingQueuedMessageRef, inlineExecutionConfig, inlinePermissionConfig, isAttachingInlineFiles, @@ -1388,17 +1418,56 @@ export function ThreadDetailPromptArea({ promptActions, promptPlaceholder, queuedComposerTextEffects, + queuedMessagePluginComposerHost, removeActiveComposerAttachment, runtimeDisplayStatus, setActiveComposerDraft, thread.id, typeaheadConfig, ]); + // The published value only ever flips between two stable host identities + // (per thread / per edit session): keystrokes do not notify the pane scope. + // While the inline editor cannot render (execution/permission configs still + // loading), the bottom composer is what is on screen, so its host stays + // published. usePublishPluginComposerHost( - queuedMessageEditor?.pluginComposerHost ?? normalPluginComposerHost, + queuedMessageEditor + ? queuedMessagePluginComposerHost + : normalPluginComposerHost, ); + // Stable per edit operation like every other host: the composer config + // around it legitimately rebuilds per keystroke, but context consumers of + // the host must not re-render on identity churn. + const sentMessageEditOperationId = sentMessageEdit?.operationId ?? null; + const sentMessagePluginComposerHost = + useMemo(() => { + if (sentMessageEditOperationId === null) { + return null; + } + const operationId = sentMessageEditOperationId; + return { + scope: { kind: "thread", threadId: thread.id }, + textEffectKey: `sent-message:${thread.id}:${operationId}`, + getCurrent: () => + readSentMessageEditDraft( + sentMessageEditRef, + operationId, + ENDED_EDIT_SESSION_DRAFT, + ), + subscribeDraft: subscribeSentMessageEditDraft, + setDraft: (nextDraft) => + writeSentMessageEditDraft(sentMessageEditRef, operationId, nextDraft), + focus: focusInlinePluginComposer, + }; + }, [ + focusInlinePluginComposer, + sentMessageEditOperationId, + sentMessageEditRef, + subscribeSentMessageEditDraft, + thread.id, + ]); const sentMessageEditorPortal = useMemo(() => { - if (!sentMessageEdit?.hostElement) { + if (!sentMessageEdit?.hostElement || !sentMessagePluginComposerHost) { return null; } const { draft, hostElement, operationId } = sentMessageEdit; @@ -1444,20 +1513,7 @@ export function ThreadDetailPromptArea({ onSelectHistoryEntry: (nextDraft) => sentMessageEdit.updateDraft(() => nextDraft), permission: bottomPermissionConfig, - pluginComposerHost: { - scope: { kind: "thread", threadId: thread.id }, - textEffectKey: `sent-message:${thread.id}:${operationId}`, - draft, - getCurrent: () => - readSentMessageEditDraft(sentMessageEditRef, operationId, draft), - setDraft: (nextDraft) => - writeSentMessageEditDraft( - sentMessageEditRef, - operationId, - nextDraft, - ), - focus: focusInlinePluginComposer, - }, + pluginComposerHost: sentMessagePluginComposerHost, promptActions, promptPlaceholder: "Edit message", submit: handleSentMessageEditSubmit, @@ -1477,7 +1533,6 @@ export function ThreadDetailPromptArea({ canSubmitSentMessageEdit, compactExecutionConfig, editFocusNonce, - focusInlinePluginComposer, handleAttachSentMessageFiles, handleSentMessageEditSubmit, isAttachingSentMessageFiles, @@ -1487,8 +1542,8 @@ export function ThreadDetailPromptArea({ sentMessageAttachmentError, sentMessageComposerTextEffects, sentMessageEdit, - sentMessageEditRef, sentMessageEditSubmitMode, + sentMessagePluginComposerHost, thread.id, typeaheadConfig, ]); @@ -1588,7 +1643,7 @@ export function ThreadDetailPromptArea({ ({ + timelinePaneRenders: vi.fn(), +})); + vi.mock("./ThreadTimelinePane", async (importOriginal) => { const React = await import("react"); const actual = await importOriginal(); @@ -148,8 +162,9 @@ vi.mock("./ThreadTimelinePane", async (importOriginal) => { const ThreadTimelinePane = ({ footer, threadId, - }: ComponentProps) => - React.createElement( + }: ComponentProps) => { + timelinePaneRenders(); + return React.createElement( "div", { "data-testid": "thread-timeline-pane", @@ -157,6 +172,7 @@ vi.mock("./ThreadTimelinePane", async (importOriginal) => { }, footer, ); + }; return { ...actual, ThreadTimelinePane }; }); @@ -172,6 +188,44 @@ const hostedPaneRegistration = { }, }; +/** + * Mirrors ThreadDetailPromptArea's host construction over the real draft + * store: an identity-stable host publishing into the pane scope from the + * footer slot, plus an actual draft consumer, exactly where the composer + * sits in the app tree. + */ +function FooterComposerHostPublisher({ threadId }: { threadId: string }) { + const promptDraft = usePromptDraftStorage({ + kind: "thread", + projectId: "proj-test", + threadId, + }); + const host = useMemo( + () => ({ + scope: { kind: "thread", threadId }, + textEffectKey: promptDraft.storageKey, + getCurrent: promptDraft.getCurrent, + subscribeDraft: promptDraft.subscribe, + setDraft: promptDraft.setDraft, + focus: () => {}, + }), + [ + promptDraft.getCurrent, + promptDraft.setDraft, + promptDraft.storageKey, + promptDraft.subscribe, + threadId, + ], + ); + usePublishPluginComposerHost(host); + return ; +} + +function FooterComposerDraftProbe() { + const draft = usePluginComposerHostDraft(usePluginComposerHost()); + return
{draft?.text ?? ""}
; +} + function makeThread(): ThreadDetailSecondaryContentProps["metadata"]["thread"] { return { archivedAt: null, @@ -313,7 +367,9 @@ afterEach(() => { cleanup(); publishedHostedPanel = null; secondaryPanelMockState.renderBrowserDeck = undefined; + timelinePaneRenders.mockClear(); useThreadsMock.mockClear(); + window.localStorage.clear(); }); // The secondary panel chunk loads lazily, so the panel appears one tick @@ -461,4 +517,45 @@ describe("ThreadDetailSecondaryContent", () => { enabled: true, }); }); + + // Regression probe for the 19.6s mobile keystroke hang: the body holds the + // published composer host, so a per-keystroke host identity re-rendered + // SecondaryPanelLayout and the timeline pane per character. + it("does not re-render the timeline pane while the composer draft changes", async () => { + const props = createProps(); + props.footer = ; + render( + + + + + + + , + ); + + // Let the lazy secondary panel and the host publication settle first. + await screen.findByTestId("inline-secondary-panel", {}, { timeout: 5_000 }); + expect(screen.getByTestId("footer-composer-draft").textContent).toBe(""); + const paneRendersAfterMount = timelinePaneRenders.mock.calls.length; + + // Keystrokes hit the same store the composer writes through. + const accessor = getPromptDraftAccessor({ + kind: "thread", + projectId: "proj-test", + threadId: "thread-1", + }); + const typed = "why does typing hang"; + for (let index = 1; index <= typed.length; index += 1) { + const text = typed.slice(0, index); + act(() => { + accessor.setDraft({ text, mentions: [], attachments: [] }); + }); + expect(screen.getByTestId("footer-composer-draft").textContent).toBe( + text, + ); + } + + expect(timelinePaneRenders.mock.calls.length).toBe(paneRendersAfterMount); + }); }); From 76ba0c673c82617cd76cf37b1e35d872c52c159f Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Fri, 21 Aug 2026 22:41:49 +0200 Subject: [PATCH 4/6] Cache max scroll offset outside the timeline scroll hot path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mobile telemetry shows 1,500+ scroll stalls on div.thread-scrollbar. BottomAnchoredScrollBody read scrollHeight/clientHeight on every scroll and wheel event (syncBottomStateFromScroll, markWheelScrollIntent, and the throttled writeScrollAnchor), forcing a synchronous layout pass per event on an unvirtualized timeline of up to ~5,400 DOM nodes. Mirror useStickyBottomScroll's established pattern: cache scrollHeight - clientHeight in a ref and let per-scroll-event code read only scrollTop plus the cache. The cache refreshes only where layout legitimately changes: the ResizeObserver callback (it already watches both the scroll port and the content wrapper), the bottom-restore loop (which deliberately re-reads fresh geometry after content growth), the programmatic scroll paths (scrollToBottom, clamped reveal, saved-row restore, prepend compensation), and the one-shot unmount anchor flush. Two guards keep the cache honest: - Shrink-edge verification: on a content-shrink frame the scroll event outruns the ResizeObserver refresh, so a still-pinned viewport compares its clamped scrollTop against a stale-high max and reads as a user detach — unrecoverable, since the bottom-restore is suppressed once stick-to-bottom is off (deterministic on iOS: tap-collapsing a long tool output while pinned). On the attach->detach edge only, one fresh read re-tests the predicate before flipping state; the same edge verification guards markWheelScrollIntent and writeScrollAnchor. Growth stays cache-only (stale-low is safe). Zero reads per steady-state scroll event, one per detach edge. - The cache is only authoritative after the first ResizeObserver delivery; before that (or under a polyfill that never fires, as in the shared vitest setup) hot paths fall back to live reads, the pre-cache behavior, instead of trusting a frozen value. jsdom tests now deliver the ResizeObserver notification a real browser fires when scroll geometry changes. New tests pin the contract: getter spies prove one read on the detach edge and zero across a mid-timeline burst and re-attach; a shrink-frame regression test proves a pinned viewport stays pinned and never persists a detached anchor; isAtBottom threshold transitions stay correct against the resize-refreshed cache. Co-Authored-By: Claude Fable 5 --- ...d-scroll-body.scroll-preservation.test.tsx | 234 +++++++++++++++++- .../ui/bottom-anchored-scroll-body.tsx | 203 +++++++++++---- 2 files changed, 386 insertions(+), 51 deletions(-) diff --git a/apps/app/src/components/ui/bottom-anchored-scroll-body.scroll-preservation.test.tsx b/apps/app/src/components/ui/bottom-anchored-scroll-body.scroll-preservation.test.tsx index ab5b1a1985..90cf54e3f8 100644 --- a/apps/app/src/components/ui/bottom-anchored-scroll-body.scroll-preservation.test.tsx +++ b/apps/app/src/components/ui/bottom-anchored-scroll-body.scroll-preservation.test.tsx @@ -232,13 +232,18 @@ describe("BottomAnchoredScrollBody scroll preservation", () => { top: 80, bottom: 180, }); + // Content lays out settled at the bottom; the ResizeObserver delivery + // refreshes the component's cached max scroll offset, exactly as a real + // browser does whenever the scroll port or content wrapper resizes. setScrollMetrics(scrollArea, { scrollHeight: 400, clientHeight: 100, - scrollTop: 150, + scrollTop: 300, }); + getLatestResizeObserver().trigger(); // User-intent scroll away from bottom, then a scroll event triggers capture. + scrollArea.scrollTop = 150; fireEvent.wheel(scrollArea); fireEvent.scroll(scrollArea); @@ -271,9 +276,11 @@ describe("BottomAnchoredScrollBody scroll preservation", () => { setScrollMetrics(scrollArea, { scrollHeight: 400, clientHeight: 100, - scrollTop: 150, + scrollTop: 300, }); + getLatestResizeObserver().trigger(); + scrollArea.scrollTop = 150; fireEvent.wheel(scrollArea); fireEvent.scroll(scrollArea); @@ -302,9 +309,11 @@ describe("BottomAnchoredScrollBody scroll preservation", () => { setScrollMetrics(scrollArea, { scrollHeight: 1_400, clientHeight: 100, - scrollTop: 1_000, + scrollTop: 1_300, }); + getLatestResizeObserver().trigger(); + scrollArea.scrollTop = 1_000; fireEvent.wheel(scrollArea); fireEvent.scroll(scrollArea); @@ -330,9 +339,11 @@ describe("BottomAnchoredScrollBody scroll preservation", () => { setScrollMetrics(scrollArea, { scrollHeight: 400, clientHeight: 100, - scrollTop: 150, + scrollTop: 300, }); + getLatestResizeObserver().trigger(); + scrollArea.scrollTop = 150; fireEvent.wheel(scrollArea, { deltaY: -100 }); fireEvent.scroll(scrollArea); fireEvent.click(getByRole("button", { name: "Capture prepend anchor" })); @@ -400,6 +411,7 @@ describe("BottomAnchoredScrollBody scroll preservation", () => { clientHeight: 100, scrollTop: 300, }); + getLatestResizeObserver().trigger(); fireEvent.scroll(scrollArea); @@ -641,8 +653,10 @@ describe("BottomAnchoredScrollBody scroll preservation", () => { setScrollMetrics(a1.scrollArea, { scrollHeight: 400, clientHeight: 100, - scrollTop: 150, + scrollTop: 300, }); + getLatestResizeObserver().trigger(); + a1.scrollArea.scrollTop = 150; fireEvent.wheel(a1.scrollArea); fireEvent.scroll(a1.scrollArea); a1.unmount(); @@ -664,8 +678,10 @@ describe("BottomAnchoredScrollBody scroll preservation", () => { setScrollMetrics(b.scrollArea, { scrollHeight: 400, clientHeight: 100, - scrollTop: 150, + scrollTop: 300, }); + getLatestResizeObserver().trigger(); + b.scrollArea.scrollTop = 150; fireEvent.wheel(b.scrollArea); fireEvent.scroll(b.scrollArea); b.unmount(); @@ -702,4 +718,210 @@ describe("BottomAnchoredScrollBody scroll preservation", () => { expect(a2.scrollArea.scrollTop).toBe(220); }); + + it("never reads scrollHeight or clientHeight from per-scroll-event handlers", () => { + const { scrollArea, rowElements } = renderTimeline({ + threadId: "thread-a", + rowIds: ["row-a", "row-b", "row-c"], + }); + mockScrollAreaRect(scrollArea); + mockRowRect(requireHTMLElement(rowElements.get("row-a")!), { + top: -120, + bottom: -20, + }); + mockRowRect(requireHTMLElement(rowElements.get("row-b")!), { + top: -20, + bottom: 80, + }); + mockRowRect(requireHTMLElement(rowElements.get("row-c")!), { + top: 80, + bottom: 180, + }); + setScrollMetrics(scrollArea, { + scrollHeight: 400, + clientHeight: 100, + scrollTop: 300, + }); + getLatestResizeObserver().trigger(); + + // From here on, every scrollHeight/clientHeight read is observable. On an + // unvirtualized timeline those getters force a full synchronous layout + // pass, so scroll/wheel handlers must run on the cached max offset alone. + const readScrollHeight = vi.fn(() => 400); + const readClientHeight = vi.fn(() => 100); + Object.defineProperty(scrollArea, "scrollHeight", { + configurable: true, + get: readScrollHeight, + }); + Object.defineProperty(scrollArea, "clientHeight", { + configurable: true, + get: readClientHeight, + }); + + // The attach -> detach edge is allowed exactly one verification read (the + // content-shrink guard re-testing the cached off-bottom classification). + scrollArea.scrollTop = 150; + fireEvent.wheel(scrollArea); + fireEvent.scroll(scrollArea); + expect(readScrollHeight).toHaveBeenCalledTimes(1); + expect(readClientHeight).toHaveBeenCalledTimes(1); + readScrollHeight.mockClear(); + readClientHeight.mockClear(); + + // Steady state — a mid-timeline scroll burst, a wheel-down, and a return + // to the bottom — must be entirely read-free. + for (let scrollTop = 140; scrollTop >= 50; scrollTop -= 10) { + scrollArea.scrollTop = scrollTop; + fireEvent.scroll(scrollArea); + } + fireEvent.wheel(scrollArea, { deltaY: 120 }); + scrollArea.scrollTop = 300; + fireEvent.scroll(scrollArea); + + expect(readScrollHeight).not.toHaveBeenCalled(); + expect(readClientHeight).not.toHaveBeenCalled(); + // The cached geometry still classified the burst correctly: the detach + // captured the top-most visible row mid-timeline... + expect(readAnchor("thread-a")).toEqual({ + rowId: "row-b", + offsetWithinRow: 20, + atBottom: false, + }); + // ...and the final scroll re-attached to the bottom, so the next growth + // (a legitimate fresh read in the resize path) restores to the new max. + setScrollMetrics(scrollArea, { + scrollHeight: 500, + clientHeight: 100, + scrollTop: 300, + }); + getLatestResizeObserver().trigger(); + expect(scrollArea.scrollTop).toBe(400); + }); + + it("stays pinned when a shrink-frame scroll event outruns the resize refresh", () => { + const { scrollArea, rowElements } = renderTimeline({ + threadId: "thread-a", + rowIds: ["row-a", "row-b", "row-c"], + }); + mockScrollAreaRect(scrollArea); + mockRowRect(requireHTMLElement(rowElements.get("row-a")!), { + top: -120, + bottom: -20, + }); + mockRowRect(requireHTMLElement(rowElements.get("row-b")!), { + top: -20, + bottom: 80, + }); + // Pinned at the bottom of settled content; the cached max offset is 300. + setScrollMetrics(scrollArea, { + scrollHeight: 400, + clientHeight: 100, + scrollTop: 300, + }); + getLatestResizeObserver().trigger(); + + // iOS tap-collapsing a long tool output while pinned: the touch marks + // user intent, the content shrinks (new max offset 100), and the browser + // clamps scrollTop and delivers the scroll event BEFORE the + // ResizeObserver refresh — the cache still says 300. + fireEvent.touchStart(scrollArea); + setScrollMetrics(scrollArea, { + scrollHeight: 200, + clientHeight: 100, + scrollTop: 100, + }); + fireEvent.scroll(scrollArea); + + // The stale-high cache reads 200px off-bottom with recent intent, which + // would detach for good (the bottom-restore is suppressed once + // stick-to-bottom is off) and persist a mid-timeline row anchor. The + // detach-edge verification must keep us pinned instead. + expect(readAnchor("thread-a")).toEqual({ + rowId: "", + offsetWithinRow: 0, + atBottom: true, + }); + + // The late resize delivery finds stick-to-bottom intact, so further + // content growth keeps following the bottom. + getLatestResizeObserver().trigger(); + setScrollMetrics(scrollArea, { + scrollHeight: 250, + clientHeight: 100, + scrollTop: 100, + }); + getLatestResizeObserver().trigger(); + expect(scrollArea.scrollTop).toBe(150); + }); + + it("tracks isAtBottom transitions against the cache refreshed by resizes", () => { + const { scrollArea, rowElements } = renderTimeline({ + threadId: "thread-a", + rowIds: ["row-a", "row-b", "row-c"], + }); + mockScrollAreaRect(scrollArea); + mockRowRect(requireHTMLElement(rowElements.get("row-a")!), { + top: -120, + bottom: -20, + }); + mockRowRect(requireHTMLElement(rowElements.get("row-b")!), { + top: -20, + bottom: 80, + }); + setScrollMetrics(scrollArea, { + scrollHeight: 400, + clientHeight: 100, + scrollTop: 300, + }); + getLatestResizeObserver().trigger(); + + // Detach: against the cached max offset (300), 100 is far off the bottom. + scrollArea.scrollTop = 100; + fireEvent.wheel(scrollArea); + fireEvent.scroll(scrollArea); + expect(readAnchor("thread-a")).toEqual({ + rowId: "row-b", + offsetWithinRow: 20, + atBottom: false, + }); + + // Content doubles while detached: the resize refreshes the cache (max + // offset 700) without yanking the detached viewport. + setScrollMetrics(scrollArea, { + scrollHeight: 800, + clientHeight: 100, + scrollTop: 100, + }); + getLatestResizeObserver().trigger(); + expect(scrollArea.scrollTop).toBe(100); + + // 694 is 6px shy of the refreshed bottom (700), outside the 4px threshold: + // still detached. The stale pre-resize max (300) would misclassify it as + // at-bottom and the growth below would yank to the new maximum. + fireEvent.wheel(scrollArea, { deltaY: 400 }); + scrollArea.scrollTop = 694; + fireEvent.scroll(scrollArea); + + setScrollMetrics(scrollArea, { + scrollHeight: 900, + clientHeight: 100, + scrollTop: 694, + }); + getLatestResizeObserver().trigger(); + expect(scrollArea.scrollTop).toBe(694); + + // 797 is 3px shy of the again-refreshed bottom (800): re-attaches... + fireEvent.wheel(scrollArea, { deltaY: 200 }); + scrollArea.scrollTop = 797; + fireEvent.scroll(scrollArea); + + // ...so the next content growth follows the bottom again. + setScrollMetrics(scrollArea, { + scrollHeight: 1_000, + clientHeight: 100, + scrollTop: 797, + }); + getLatestResizeObserver().trigger(); + expect(scrollArea.scrollTop).toBe(900); + }); }); diff --git a/apps/app/src/components/ui/bottom-anchored-scroll-body.tsx b/apps/app/src/components/ui/bottom-anchored-scroll-body.tsx index b7e86e432d..eee0a5e491 100644 --- a/apps/app/src/components/ui/bottom-anchored-scroll-body.tsx +++ b/apps/app/src/components/ui/bottom-anchored-scroll-body.tsx @@ -129,19 +129,15 @@ export function useBottomAnchoredScroll(): BottomAnchorContextValue | null { return useContext(BottomAnchorContext); } +// Reading `scrollHeight`/`clientHeight` forces synchronous layout (WebKit +// especially), so only the cache-refresh paths may call this — never +// per-scroll-event code. See `refreshMaxScrollOffset` in the component. function getMaxScrollOffset(element: HTMLElement) { return Math.max(0, element.scrollHeight - element.clientHeight); } -function isScrolledNearBottom(element: HTMLElement) { - return ( - getMaxScrollOffset(element) - element.scrollTop <= - BOTTOM_ANCHOR_THRESHOLD_PX - ); -} - -function scrollElementToBottom(element: HTMLElement) { - element.scrollTop = getMaxScrollOffset(element); +function isScrolledNearBottom(maxScrollOffset: number, scrollTop: number) { + return maxScrollOffset - scrollTop <= BOTTOM_ANCHOR_THRESHOLD_PX; } function isElementFullyVisibleInScrollArea({ @@ -168,13 +164,6 @@ function getScrollOffsetToRevealElement({ ); } -function getRevealScrollOffsetClampedToMax(args: ElementVisibilityArgs) { - return Math.min( - getMaxScrollOffset(args.scrollArea), - getScrollOffsetToRevealElement(args), - ); -} - interface TopMostVisibleRow { rowId: string; offsetWithinRow: number; @@ -315,6 +304,23 @@ export function BottomAnchoredScrollBody({ trailingTimeout: number | null; }>({ lastWriteAt: 0, trailingTimeout: null }); const userDetachedFromBottomRef = useRef(false); + // Cached `scrollHeight - clientHeight` of the scroll area. Reading those two + // properties forces synchronous layout in WebKit, which on an unvirtualized + // timeline (thousands of DOM nodes) stalls every scroll event. Mirroring + // useStickyBottomScroll, per-scroll-event handlers read only `scrollTop` + // plus this cache; it is refreshed where layout legitimately changes — the + // ResizeObserver (which watches both the scroll port and the content + // wrapper, so every size change lands there), the programmatic + // scroll/restore paths, which need fresh geometry anyway, and one fresh + // verification read on the attach->detach edge (see + // syncBottomStateFromScroll for the content-shrink race it covers). + const maxScrollOffsetRef = useRef(0); + // The cache is only authoritative once the ResizeObserver has delivered: + // without deliveries (no ResizeObserver in the environment, or a no-op + // polyfill that never fires) nothing keeps it fresh, so reads fall back to + // live geometry — the pre-cache behavior — instead of trusting a frozen + // value that would classify every position as at-bottom. + const resizeObserverHasDeliveredRef = useRef(false); const [isAtBottom, setIsAtBottom] = useState(true); const initialScrollRestoreRowId = useMemo(() => { if (scrollAnchorThreadId === undefined) return null; @@ -328,6 +334,22 @@ export function BottomAnchoredScrollBody({ const getScrollElement = useCallback(() => scrollAreaRef.current, []); + const refreshMaxScrollOffset = useCallback((scrollArea: HTMLElement) => { + const maxScrollOffset = getMaxScrollOffset(scrollArea); + maxScrollOffsetRef.current = maxScrollOffset; + return maxScrollOffset; + }, []); + + // Hot-path read: the cache once the ResizeObserver has delivered, a live + // read before that (and forever, when no observer will ever fire). + const readMaxScrollOffset = useCallback( + (scrollArea: HTMLElement) => + resizeObserverHasDeliveredRef.current + ? maxScrollOffsetRef.current + : refreshMaxScrollOffset(scrollArea), + [refreshMaxScrollOffset], + ); + const cancelPendingScrollRestore = useCallback(() => { pendingScrollRestoreRef.current = null; }, []); @@ -344,19 +366,25 @@ export function BottomAnchoredScrollBody({ // once we're pinned again. // // CSS scroll anchoring (the trailing sentinel) keeps scrollTop pinned at - // sub-pixel precision during content growth/shrink. `scrollElementToBottom` - // sets `scrollTop = scrollHeight - clientHeight` — both integer-rounded - // Web API values — so calling it while we're already within sub-pixel - // range yanks scrollTop by ±1px against the browser's fractional value, - // producing visible jitter on every frame of a row expand/collapse. - // Restore only when anchoring has actually let us drift away from bottom. + // sub-pixel precision during content growth/shrink. Setting + // `scrollTop = scrollHeight - clientHeight` — both integer-rounded Web API + // values — while we're already within sub-pixel range yanks scrollTop by + // ±1px against the browser's fractional value, producing visible jitter on + // every frame of a row expand/collapse. Restore only when anchoring has + // actually let us drift away from bottom. + // + // This runs after observed size changes, so it deliberately reads fresh + // geometry — layout has genuinely changed — and refreshes the cache with it. const restoreBottomOnce = useCallback(() => { const scrollArea = scrollAreaRef.current; if (!scrollArea || !shouldStickToBottomRef.current) return false; - if (isScrolledNearBottom(scrollArea)) return false; - scrollElementToBottom(scrollArea); + const maxScrollOffset = refreshMaxScrollOffset(scrollArea); + if (isScrolledNearBottom(maxScrollOffset, scrollArea.scrollTop)) { + return false; + } + scrollArea.scrollTop = maxScrollOffset; return true; - }, []); + }, [refreshMaxScrollOffset]); const queueBottomRestore = useCallback(() => { if (!shouldStickToBottomRef.current) return; @@ -398,10 +426,10 @@ export function BottomAnchoredScrollBody({ shouldStickToBottomRef.current = true; setIsAtBottom(true); if (scrollArea) { - scrollElementToBottom(scrollArea); + scrollArea.scrollTop = refreshMaxScrollOffset(scrollArea); } queueBottomRestore(); - }, [cancelPendingScrollRestore, queueBottomRestore]); + }, [cancelPendingScrollRestore, queueBottomRestore, refreshMaxScrollOffset]); const scrollElementIntoView = useCallback( ({ element, options }: ScrollElementIntoViewArgs) => { @@ -428,12 +456,16 @@ export function BottomAnchoredScrollBody({ return; } - scrollArea.scrollTop = getRevealScrollOffsetClampedToMax({ - element, - scrollArea, - }); + const maxScrollOffset = refreshMaxScrollOffset(scrollArea); + scrollArea.scrollTop = Math.min( + maxScrollOffset, + getScrollOffsetToRevealElement({ element, scrollArea }), + ); - const targetIsAtBottom = isScrolledNearBottom(scrollArea); + const targetIsAtBottom = isScrolledNearBottom( + maxScrollOffset, + scrollArea.scrollTop, + ); shouldStickToBottomRef.current = targetIsAtBottom; setIsAtBottom(targetIsAtBottom); @@ -444,7 +476,7 @@ export function BottomAnchoredScrollBody({ cancelQueuedRestore(); }, - [cancelQueuedRestore, queueBottomRestore], + [cancelQueuedRestore, queueBottomRestore, refreshMaxScrollOffset], ); const captureScrollAnchor = useCallback(() => { @@ -464,6 +496,9 @@ export function BottomAnchoredScrollBody({ if (delta <= 0) return; scrollArea.scrollTop = anchor.scrollTop + delta; pendingPrependAnchorRef.current = null; + // Content height just changed under us; fold it into the cache now instead + // of waiting for the ResizeObserver delivery at the end of the frame. + refreshMaxScrollOffset(scrollArea); }); const hasRecentUserScrollIntent = useCallback(() => { @@ -483,7 +518,19 @@ export function BottomAnchoredScrollBody({ if (scrollAnchorThreadId === undefined) return; const scrollArea = scrollAreaOverride ?? scrollAreaRef.current; if (!scrollArea) return; - const atBottomByGeometry = isScrolledNearBottom(scrollArea); + let atBottomByGeometry = isScrolledNearBottom( + readMaxScrollOffset(scrollArea), + scrollArea.scrollTop, + ); + if (!atBottomByGeometry && shouldStickToBottomRef.current) { + // Same content-shrink edge as syncBottomStateFromScroll: while still + // attached, verify an off-bottom reading with fresh geometry before + // letting it demote the anchor to a mid-timeline row. + atBottomByGeometry = isScrolledNearBottom( + refreshMaxScrollOffset(scrollArea), + scrollArea.scrollTop, + ); + } const recentUserIntent = hasRecentUserScrollIntent(); const anchorAtom = threadTimelineScrollAnchorAtomFamily(scrollAnchorThreadId); @@ -519,7 +566,13 @@ export function BottomAnchoredScrollBody({ atBottom: false, }); }, - [hasRecentUserScrollIntent, scrollAnchorThreadId, store], + [ + hasRecentUserScrollIntent, + readMaxScrollOffset, + refreshMaxScrollOffset, + scrollAnchorThreadId, + store, + ], ); const captureScrollAnchorThrottled = useCallback(() => { @@ -562,13 +615,13 @@ export function BottomAnchoredScrollBody({ scrollArea, }); const targetScrollTop = Math.min( - getMaxScrollOffset(scrollArea), + refreshMaxScrollOffset(scrollArea), revealOffset + anchor.offsetWithinRow, ); scrollArea.scrollTop = targetScrollTop; return targetScrollTop; }, - [cancelQueuedRestore], + [cancelQueuedRestore, refreshMaxScrollOffset], ); const markUserScrollIntent = useCallback(() => { @@ -579,13 +632,29 @@ export function BottomAnchoredScrollBody({ const markWheelScrollIntent = useCallback( (event: WheelEvent) => { const scrollArea = scrollAreaRef.current; - if (event.deltaY > 0 && scrollArea && isScrolledNearBottom(scrollArea)) { - userScrollIntentUntilRef.current = 0; - return; + // Wheel events fire at scroll rate; run on the cached max offset and + // spend a fresh verification read only when a still-attached viewport + // reads as off-bottom (the content-shrink edge described in + // syncBottomStateFromScroll). While detached, wheeling stays cache-only. + if (event.deltaY > 0 && scrollArea) { + const nearBottom = + isScrolledNearBottom( + readMaxScrollOffset(scrollArea), + scrollArea.scrollTop, + ) || + (shouldStickToBottomRef.current && + isScrolledNearBottom( + refreshMaxScrollOffset(scrollArea), + scrollArea.scrollTop, + )); + if (nearBottom) { + userScrollIntentUntilRef.current = 0; + return; + } } markUserScrollIntent(); }, - [markUserScrollIntent], + [markUserScrollIntent, readMaxScrollOffset, refreshMaxScrollOffset], ); const markTouchStartScrollIntent = useCallback(() => { @@ -637,7 +706,33 @@ export function BottomAnchoredScrollBody({ return; } - if (isScrolledNearBottom(scrollArea)) { + // Cached max offset: this runs on every scroll event of an unvirtualized + // timeline, where a scrollHeight/clientHeight read would force a full + // layout pass per event. + let nearBottom = isScrolledNearBottom( + readMaxScrollOffset(scrollArea), + scrollArea.scrollTop, + ); + if ( + !nearBottom && + shouldStickToBottomRef.current && + hasRecentUserScrollIntent() + ) { + // Attach -> detach edge. On a content-shrink frame this scroll event + // outruns the ResizeObserver refresh: the browser has already clamped + // scrollTop to the new, smaller maximum while the cache still holds the + // old one, so a still-pinned viewport reads as a user detach — with no + // recovery, because the bottom-restore is suppressed once + // stick-to-bottom is off (deterministic on iOS, e.g. tap-collapsing a + // long tool output while pinned). Spend one fresh read on this edge + // only to re-test; steady-state scrolling stays cache-only. + nearBottom = isScrolledNearBottom( + refreshMaxScrollOffset(scrollArea), + scrollArea.scrollTop, + ); + } + + if (nearBottom) { userDetachedFromBottomRef.current = false; shouldStickToBottomRef.current = true; userScrollIntentUntilRef.current = 0; @@ -656,7 +751,12 @@ export function BottomAnchoredScrollBody({ cancelQueuedRestore(); // The user is scrolling on their own; don't yank them back to the anchor. pendingScrollRestoreRef.current = null; - }, [cancelQueuedRestore, hasRecentUserScrollIntent]); + }, [ + cancelQueuedRestore, + hasRecentUserScrollIntent, + readMaxScrollOffset, + refreshMaxScrollOffset, + ]); const handleScroll = useCallback(() => { syncBottomStateFromScroll(); @@ -698,11 +798,21 @@ export function BottomAnchoredScrollBody({ }, [applyScrollRestore, queueBottomRestore]); const handleScrollAreaResize = useCallback(() => { + const scrollArea = scrollAreaRef.current; + if (scrollArea) { + // The steady-state cache refresh: the observer watches both the scroll + // port and the content wrapper, so every legitimate + // scrollHeight/clientHeight change passes through here. The first + // delivery is also what makes the cache authoritative for hot-path + // reads (see resizeObserverHasDeliveredRef). + refreshMaxScrollOffset(scrollArea); + resizeObserverHasDeliveredRef.current = true; + } // While a restore is pending, the ResizeObserver is the settle signal; the // bottom-restore is suppressed (stick-to-bottom is false) anyway. if (advancePendingScrollRestore()) return; queueBottomRestore(); - }, [advancePendingScrollRestore, queueBottomRestore]); + }, [advancePendingScrollRestore, queueBottomRestore, refreshMaxScrollOffset]); // Begin restoring the saved scroll position on mount, before the listener // effect's `queueBottomRestore()` runs (a useEffect, which runs after layout @@ -752,9 +862,12 @@ export function BottomAnchoredScrollBody({ window.clearTimeout(captureThrottle.trailingTimeout); captureThrottle.trailingTimeout = null; } + // One-shot unmount flush: the cache may lag the final layout by a frame + // and this write decides where the user comes back to, so read fresh. + refreshMaxScrollOffset(scrollArea); writeScrollAnchor(scrollArea); }, - [writeScrollAnchor], + [refreshMaxScrollOffset, writeScrollAnchor], ); useLayoutEffect(() => { From 8e8ae24327445e0cdd9d58fa54f25754458e835b Mon Sep 17 00:00:00 2001 From: Vedran Burojevic Date: Fri, 21 Aug 2026 22:35:12 +0200 Subject: [PATCH 5/6] Apply urgent realtime thread changes without flushing the debounce buffer Any thread message containing an immediate change kind (status-changed, history-rewritten, environment-changed, tabs-changed) flushed the entire buffered invalidation state, including every debounced events-appended timeline invalidation for every thread. The queued-message send path publishes [events-appended, queue-changed, status-changed] as one message, so during streaming the 50/200ms coalescing window collapsed exactly when it mattered, and one thread's status flip flushed every other streaming thread's timeline invalidations with it. Partition each message by the registry's flush priority: immediate kinds run their dirty handlers synchronously against that message alone, and debounced kinds stay in the scheduler. Turn completion is exempt: when a message's eventTypes include turn/completed, every kind is recorded and the buffer flushes at once (the old path), because the lifecycle publish bundles the final events-appended with the status flip and splitting them would re-enable the composer up to a debounce window before the final assistant text renders; a completed stream needs no coalescing protection. Hidden-document deferral is unchanged. The immediate path routes message metadata through mergeThreadChangeMetadata so its context (including #2169's statusChange row snapshot) cannot drift from the buffered flush path, and passes metadata only when the message carries a thread id: an id-less global message runs its handlers with undefined metadata exactly like the flush's global path, so a stray projectId cannot narrow dirtyActiveThreadListQueries to one project. Behavior change: only mid-stream bundles are partitioned, so a status flip applies immediately while the timeline invalidation keeps its coalescing window (up to 50ms); the turn-completion supersede stays atomic. The "once per flush" search test now drives its two completions through the hidden-document merge, the one place completions still coalesce into a single flush. Co-Authored-By: Claude Fable 5 --- .../cache-owners/realtime-cache-registry.ts | 28 ++- .../src/hooks/realtime-cache-effects.test.ts | 214 +++++++++++++++++- apps/app/src/hooks/realtime-cache-effects.ts | 86 ++++++- 3 files changed, 314 insertions(+), 14 deletions(-) diff --git a/apps/app/src/hooks/cache-owners/realtime-cache-registry.ts b/apps/app/src/hooks/cache-owners/realtime-cache-registry.ts index dc7943bab0..a4a849dee4 100644 --- a/apps/app/src/hooks/cache-owners/realtime-cache-registry.ts +++ b/apps/app/src/hooks/cache-owners/realtime-cache-registry.ts @@ -736,12 +736,30 @@ export function executeRealtimeDirtyHandlers< } } -export function shouldFlushThreadChangesImmediately( +export interface ThreadChangesByFlushPriority { + debounced: ThreadChangeKind[] | null; + immediate: ThreadChangeKind[] | null; +} + +/** + * Split a message's change kinds by the registry's flush priority. Streaming + * publishes bundle debounced kinds with immediate ones (events-appended rides + * with status-changed), so callers must apply the immediate kinds alone + * instead of flushing every buffered invalidation along with them. + */ +export function partitionThreadChangesByFlushPriority( changes: readonly ThreadChangeKind[], -): boolean { - return changes.some( - (change) => REALTIME_THREAD_CHANGE_REGISTRY[change].flush === "immediate", - ); +): ThreadChangesByFlushPriority { + let debounced: ThreadChangeKind[] | null = null; + let immediate: ThreadChangeKind[] | null = null; + for (const change of changes) { + if (REALTIME_THREAD_CHANGE_REGISTRY[change].flush === "immediate") { + (immediate ??= []).push(change); + } else { + (debounced ??= []).push(change); + } + } + return { debounced, immediate }; } export function collectCachedThreadIdsForEnvironment({ diff --git a/apps/app/src/hooks/realtime-cache-effects.test.ts b/apps/app/src/hooks/realtime-cache-effects.test.ts index 1f670f91e9..284b7e378e 100644 --- a/apps/app/src/hooks/realtime-cache-effects.test.ts +++ b/apps/app/src/hooks/realtime-cache-effects.test.ts @@ -507,7 +507,9 @@ describe("createRealtimeCacheEffects", () => { it("refreshes an open search once per flush without aborting the request in flight", async () => { vi.useFakeTimers(); - const { effects, queryClient } = createRealtimeEffectsTestContext(); + const visibility = createFakeVisibility(); + const { effects, queryClient } = + createRealtimeEffectsTestContext(visibility); const threadSearchKey = threadSearchQueryKey({ limitPerGroup: 20, query: "needle", @@ -530,7 +532,10 @@ describe("createRealtimeCacheEffects", () => { expect(searchQueryFn).toHaveBeenCalledTimes(1); const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries"); - // Two threads complete a turn inside one debounce window. + // A visible completion flushes on arrival, so two completions only share + // a flush where they still coalesce: merged behind a hidden document and + // replayed as one flush on resume. + visibility.setVisible(false); for (const threadId of ["thr_1", "thr_2"]) { effects.handleChanged({ type: "changed", @@ -540,7 +545,8 @@ describe("createRealtimeCacheEffects", () => { changes: ["events-appended"], }); } - await vi.advanceTimersByTimeAsync(50); + visibility.setVisible(true); + await vi.advanceTimersByTimeAsync(0); const searchInvalidations = invalidateSpy.mock.calls.filter( ([filters]) => @@ -1416,6 +1422,125 @@ describe("createRealtimeCacheEffects", () => { effects.dispose(); }); + it("keeps timeline invalidations debounced when status-changed rides the same publish", () => { + vi.useFakeTimers(); + const { effects, queryClient } = createRealtimeEffectsTestContext(); + const threadKey = threadQueryKey("thr_1"); + const timelineKey = threadTimelineQueryKey("thr_1"); + queryClient.setQueryData(threadKey, { id: "thr_1" }); + queryClient.setQueryData(timelineKey, { + rows: [], + timelinePage: { + kind: "latest", + topLevelLimit: 100, + returnedOlderTopLevelRowCount: 0, + hasOlderRows: false, + olderCursor: null, + }, + }); + + // The queued-message send path publishes this exact bundle per batch. + effects.handleChanged({ + type: "changed", + entity: "thread", + id: "thr_1", + metadata: { projectId: "project-1" }, + changes: ["events-appended", "queue-changed", "status-changed"], + }); + + // The urgent status flip applies synchronously… + expect(queryClient.getQueryState(threadKey)?.isInvalidated).toBe(true); + // …without dragging the timeline invalidation out of its window. + expect(queryClient.getQueryState(timelineKey)?.isInvalidated).not.toBe( + true, + ); + + vi.advanceTimersByTime(50); + expect(queryClient.getQueryState(timelineKey)?.isInvalidated).toBe(true); + + effects.dispose(); + }); + + it("leaves another thread's buffered invalidations debounced when a status flip flushes", () => { + vi.useFakeTimers(); + const { effects, queryClient } = createRealtimeEffectsTestContext(); + const streamingTimelineKey = threadTimelineQueryKey("thr_streaming"); + const flippedThreadKey = threadQueryKey("thr_flipped"); + queryClient.setQueryData(flippedThreadKey, { id: "thr_flipped" }); + queryClient.setQueryData(streamingTimelineKey, { + rows: [], + timelinePage: { + kind: "latest", + topLevelLimit: 100, + returnedOlderTopLevelRowCount: 0, + hasOlderRows: false, + olderCursor: null, + }, + }); + + effects.handleChanged({ + type: "changed", + entity: "thread", + id: "thr_streaming", + metadata: { eventTypes: ["item/agentMessage/delta"] }, + changes: ["events-appended"], + }); + effects.handleChanged({ + type: "changed", + entity: "thread", + id: "thr_flipped", + changes: ["status-changed"], + }); + + expect(queryClient.getQueryState(flippedThreadKey)?.isInvalidated).toBe( + true, + ); + expect( + queryClient.getQueryState(streamingTimelineKey)?.isInvalidated, + ).not.toBe(true); + + vi.advanceTimersByTime(50); + expect(queryClient.getQueryState(streamingTimelineKey)?.isInvalidated).toBe( + true, + ); + + effects.dispose(); + }); + + it("applies an id-less immediate change with undefined metadata like the global flush path", () => { + vi.useFakeTimers(); + const { effects, queryClient } = createRealtimeEffectsTestContext(); + const projectAListKey = threadListQueryKey({ + archived: false, + projectId: "project-a", + }); + const projectBListKey = threadListQueryKey({ + archived: false, + projectId: "project-b", + }); + queryClient.setQueryData(projectAListKey, []); + queryClient.setQueryData(projectBListKey, []); + + // A global status-changed (no thread id) dirties every project's lists, + // exactly like the flush's global path. A projectId riding the message + // metadata must not narrow the invalidation to that one project. + effects.handleChanged({ + type: "changed", + entity: "thread", + metadata: { projectId: "project-a" }, + changes: ["status-changed"], + }); + + expect(queryClient.getQueryState(projectAListKey)?.isInvalidated).toBe( + true, + ); + expect(queryClient.getQueryState(projectBListKey)?.isInvalidated).toBe( + true, + ); + + effects.dispose(); + }); + it("invalidates timeline but not thread detail or prompt history for non-turn-request events", () => { vi.useFakeTimers(); const { effects, queryClient } = createRealtimeEffectsTestContext(); @@ -1619,7 +1744,9 @@ describe("createRealtimeCacheEffects", () => { expect(signals[0]?.aborted).toBe(false); // The server sends events-appended before its immediate status-changed - // notification. The latter flushes both buffered changes together. + // notification. A completed stream needs no coalescing protection, so + // the completion event flushes the buffer at once instead of waiting + // for the debounce window; the bare status flip then applies alone. effects.handleChanged({ type: "changed", entity: "thread", @@ -1955,6 +2082,85 @@ describe("createRealtimeCacheEffects", () => { effects.dispose(); }); + it("refetches over a patched row when a bare status-changed arrives while visible", async () => { + // Stop requests, command failures and host interruptions push the bare + // kind. On the visible path status-changed never enters the debounce + // buffer: it applies immediately, and without a row snapshot it must + // fall back to the refetch so an earlier patched status cannot go stale. + vi.useFakeTimers(); + const { effects, queryClient } = createRealtimeEffectsTestContext(); + const sidebarNavigationKey = sidebarNavigationQueryKey(); + const idleRow = { + activity: NO_THREAD_ACTIVITY, + id: "thr_1", + latestAttentionAt: 100, + runtime: { displayStatus: "idle", hostReconnectGraceExpiresAt: null }, + status: "idle", + updatedAt: 100, + }; + const stoppedRow = { ...idleRow, updatedAt: 300 }; + let serverRow = idleRow; + const sidebarQueryFn = vi.fn(async () => ({ + projects: [{ threads: [serverRow] }], + personalProject: { threads: [] }, + })); + const observer = new QueryObserver(queryClient, { + queryKey: sidebarNavigationKey, + queryFn: sidebarQueryFn, + staleTime: Infinity, + }); + const unsubscribe = observer.subscribe(() => {}); + await vi.advanceTimersByTimeAsync(0); + expect(sidebarQueryFn).toHaveBeenCalledTimes(1); + + effects.handleChanged({ + type: "changed", + entity: "thread", + id: "thr_1", + metadata: { + projectId: "project-1", + statusChange: { + activity: NO_THREAD_ACTIVITY, + latestAttentionAt: 100, + runtime: { + displayStatus: "active", + hostReconnectGraceExpiresAt: null, + }, + status: "active", + updatedAt: 200, + }, + }, + changes: ["status-changed"], + }); + + // The snapshot patched the row in place, synchronously and fetch-free. + expect(sidebarQueryFn).toHaveBeenCalledTimes(1); + expect( + queryClient.getQueryData<{ + projects: { threads: (typeof idleRow)[] }[]; + }>(sidebarNavigationKey)?.projects[0]?.threads[0]?.status, + ).toBe("active"); + + serverRow = stoppedRow; + effects.handleChanged({ + type: "changed", + entity: "thread", + id: "thr_1", + changes: ["status-changed"], + }); + await vi.advanceTimersByTimeAsync(0); + + expect(sidebarQueryFn).toHaveBeenCalledTimes(2); + expect( + queryClient.getQueryData<{ + projects: { threads: (typeof idleRow)[] }[]; + }>(sidebarNavigationKey)?.projects[0]?.threads[0], + ).toEqual(stoppedRow); + + unsubscribe(); + effects.dispose(); + }); + it("restarts a sidebar fetch already in flight so its stale snapshot cannot overwrite the patched status", async () => { vi.useFakeTimers(); const { effects, queryClient } = createRealtimeEffectsTestContext(); diff --git a/apps/app/src/hooks/realtime-cache-effects.ts b/apps/app/src/hooks/realtime-cache-effects.ts index 7f7255df96..215748164a 100644 --- a/apps/app/src/hooks/realtime-cache-effects.ts +++ b/apps/app/src/hooks/realtime-cache-effects.ts @@ -31,7 +31,7 @@ import { REALTIME_PROJECT_CHANGE_REGISTRY, REALTIME_SYSTEM_CHANGE_REGISTRY, REALTIME_THREAD_CHANGE_REGISTRY, - shouldFlushThreadChangesImmediately, + partitionThreadChangesByFlushPriority, } from "./cache-owners/realtime-cache-registry"; const INVALIDATION_DEBOUNCE_MS = 50; @@ -229,6 +229,53 @@ function flushThreadInvalidations( resetThreadChangeState(state); } +interface ApplyImmediateThreadChangesArgs { + changes: readonly ThreadChangeKind[]; + id: string | undefined; + metadata: ThreadChangeMetadata | undefined; + queryClient: QueryClient; +} + +/** + * Run the dirty handlers for a message's immediate change kinds against that + * message alone. Debounced kinds buffered in {@link ThreadChangeState} stay + * untouched: an urgent status flip must not drag the expensive timeline + * invalidations out of their coalescing window, and streaming publishes + * bundle status-changed with events-appended on every batch. + */ +function applyImmediateThreadChanges({ + changes, + id, + metadata, + queryClient, +}: ApplyImmediateThreadChangesArgs): void { + // Normalize through the same merge the buffered path uses so a metadata + // field added there cannot silently diverge from the immediate path. + const merged = metadata + ? mergeThreadChangeMetadata({ + current: undefined, + next: metadata, + statusChanged: changes.includes("status-changed"), + }) + : undefined; + const flushOnce = createFlushOncePredicate(); + for (const changeKind of changes) { + executeRealtimeDirtyHandlers({ + context: { + backgroundActivityChanged: merged?.backgroundActivityChanged, + eventTypes: merged?.eventTypes, + flushOnce, + hasPendingInteraction: merged?.hasPendingInteraction, + projectId: merged?.projectId, + queryClient, + statusChange: merged?.statusChange, + threadId: id, + }, + handlers: REALTIME_THREAD_CHANGE_REGISTRY[changeKind].dirty, + }); + } +} + function recordThreadChange( state: ThreadChangeState, message: ChangedMessage, @@ -450,16 +497,45 @@ export function createRealtimeCacheEffects({ handleChanged: (message) => { const documentVisible = visibility.isDocumentVisible(); switch (message.entity) { - case "thread": - recordThreadChange(threadChangeState, message); + case "thread": { if (!documentVisible) { + recordThreadChange(threadChangeState, message); hasDeferredThreadChanges = true; - } else if (shouldFlushThreadChangesImmediately(message.changes)) { + break; + } + if (message.metadata?.eventTypes?.includes("turn/completed")) { + // Turn completion is atomic: the lifecycle publish bundles the + // final events-appended with the status flip, and partitioning + // them would re-enable the composer up to a debounce window + // before the final assistant text renders. A completed stream + // needs no coalescing protection, so record every kind and + // flush the buffer as one unit. + recordThreadChange(threadChangeState, message); invalidationScheduler.flush(); - } else { + break; + } + const { debounced, immediate } = + partitionThreadChangesByFlushPriority(message.changes); + if (debounced) { + recordThreadChange(threadChangeState, { + ...message, + changes: debounced, + }); invalidationScheduler.schedule(); } + if (immediate) { + applyImmediateThreadChanges({ + changes: immediate, + id: message.id, + // An id-less message dirties globally; its handlers must see + // undefined metadata exactly like the flush's global path, so + // a stray projectId cannot narrow the invalidation. + metadata: message.id ? message.metadata : undefined, + queryClient, + }); + } break; + } case "environment": if (!message.id) { break; From a3c074528d49b86f8a7cb026db20e1cad53e0e83 Mon Sep 17 00:00:00 2001 From: Sawyer Hood Date: Sat, 22 Aug 2026 01:43:34 -0700 Subject: [PATCH 6/6] Re-attach a detached timeline viewport that a content shrink clamps onto the bottom Follow-up to #2280 (cached max scroll offset). The PR guards the attach->detach edge with one fresh read, but not its mirror: a detached viewport that a content shrink (collapsing a long tool output near the end) clamps onto the new, smaller maximum. The browser delivers that clamp's scroll event before the ResizeObserver refresh, so the scroll handler classifies it against the stale, larger cache and keeps the viewport detached; nothing re-tests it afterwards. Before the cache, the live read re-attached on that scroll event, so streaming content kept following the bottom. Detect the case in the ResizeObserver path instead: the cache was authoritative, the max offset shrank, the viewport is detached, and it now sits within the bottom threshold. Re-attach with the same state flip the scroll handler uses (extracted as attachToBottom) and record the at-bottom anchor. A shrink that does not reach the viewport leaves it alone. Regression test fails on the #2280 head and passes here; a second test covers the non-clamping shrink. Co-Authored-By: Vedran Burojevic Co-Authored-By: Claude --- ...d-scroll-body.scroll-preservation.test.tsx | 110 ++++++++++++++++++ .../ui/bottom-anchored-scroll-body.tsx | 52 +++++++-- 2 files changed, 153 insertions(+), 9 deletions(-) diff --git a/apps/app/src/components/ui/bottom-anchored-scroll-body.scroll-preservation.test.tsx b/apps/app/src/components/ui/bottom-anchored-scroll-body.scroll-preservation.test.tsx index 90cf54e3f8..a0bd91d251 100644 --- a/apps/app/src/components/ui/bottom-anchored-scroll-body.scroll-preservation.test.tsx +++ b/apps/app/src/components/ui/bottom-anchored-scroll-body.scroll-preservation.test.tsx @@ -924,4 +924,114 @@ describe("BottomAnchoredScrollBody scroll preservation", () => { getLatestResizeObserver().trigger(); expect(scrollArea.scrollTop).toBe(900); }); + + it("re-attaches a detached viewport that a content shrink clamps onto the bottom", () => { + const { scrollArea, rowElements } = renderTimeline({ + threadId: "thread-a", + rowIds: ["row-a", "row-b", "row-c"], + }); + mockScrollAreaRect(scrollArea); + mockRowRect(requireHTMLElement(rowElements.get("row-a")!), { + top: -120, + bottom: -20, + }); + mockRowRect(requireHTMLElement(rowElements.get("row-b")!), { + top: -20, + bottom: 80, + }); + setScrollMetrics(scrollArea, { + scrollHeight: 400, + clientHeight: 100, + scrollTop: 300, + }); + getLatestResizeObserver().trigger(); + + // The user scrolls up to read: detached mid-timeline. + scrollArea.scrollTop = 150; + fireEvent.wheel(scrollArea); + fireEvent.scroll(scrollArea); + expect(readAnchor("thread-a")).toEqual({ + rowId: "row-b", + offsetWithinRow: 20, + atBottom: false, + }); + + // Tap-collapsing a long tool output below the viewport shrinks the content + // past the viewport's position: the browser clamps scrollTop onto the new + // maximum (100) and delivers that scroll event BEFORE the ResizeObserver + // refresh, so the scroll handler still classifies against the stale cache + // (300) and cannot see that the viewport now sits on the bottom. + fireEvent.touchStart(scrollArea); + setScrollMetrics(scrollArea, { + scrollHeight: 200, + clientHeight: 100, + scrollTop: 100, + }); + fireEvent.scroll(scrollArea); + + // The late resize delivery finds a detached viewport on the fresh bottom + // after a shrink and re-attaches it, exactly as the live read did before + // the cache: the anchor records at-bottom... + getLatestResizeObserver().trigger(); + expect(readAnchor("thread-a")).toEqual({ + rowId: "", + offsetWithinRow: 0, + atBottom: true, + }); + + // ...and further content growth follows the bottom again. + setScrollMetrics(scrollArea, { + scrollHeight: 250, + clientHeight: 100, + scrollTop: 100, + }); + getLatestResizeObserver().trigger(); + expect(scrollArea.scrollTop).toBe(150); + }); + + it("leaves a detached viewport alone when content shrinks without reaching it", () => { + const { scrollArea, rowElements } = renderTimeline({ + threadId: "thread-a", + rowIds: ["row-a", "row-b", "row-c"], + }); + mockScrollAreaRect(scrollArea); + mockRowRect(requireHTMLElement(rowElements.get("row-a")!), { + top: -120, + bottom: -20, + }); + mockRowRect(requireHTMLElement(rowElements.get("row-b")!), { + top: -20, + bottom: 80, + }); + setScrollMetrics(scrollArea, { + scrollHeight: 1_000, + clientHeight: 100, + scrollTop: 900, + }); + getLatestResizeObserver().trigger(); + + scrollArea.scrollTop = 200; + fireEvent.wheel(scrollArea); + fireEvent.scroll(scrollArea); + expect(readAnchor("thread-a")).toEqual({ + rowId: "row-b", + offsetWithinRow: 20, + atBottom: false, + }); + + // A collapse far below the viewport: no clamp, no scroll event, and the + // viewport (200 of a new max 500) is still well off the bottom. + setScrollMetrics(scrollArea, { + scrollHeight: 600, + clientHeight: 100, + scrollTop: 200, + }); + getLatestResizeObserver().trigger(); + expect(scrollArea.scrollTop).toBe(200); + expect(readAnchor("thread-a")).toEqual({ + rowId: "row-b", + offsetWithinRow: 20, + atBottom: false, + }); + }); }); diff --git a/apps/app/src/components/ui/bottom-anchored-scroll-body.tsx b/apps/app/src/components/ui/bottom-anchored-scroll-body.tsx index eee0a5e491..7da8501fb8 100644 --- a/apps/app/src/components/ui/bottom-anchored-scroll-body.tsx +++ b/apps/app/src/components/ui/bottom-anchored-scroll-body.tsx @@ -686,6 +686,19 @@ export function BottomAnchoredScrollBody({ [markUserScrollIntent], ); + // Resume following the bottom. Shared by the scroll handler (a scroll event + // landing near the bottom) and the resize path (a content shrink clamping a + // detached viewport onto the bottom). + const attachToBottom = useCallback(() => { + userDetachedFromBottomRef.current = false; + shouldStickToBottomRef.current = true; + userScrollIntentUntilRef.current = 0; + setIsAtBottom(true); + // A deliberate arrival at the bottom during the restore settle window means + // the user no longer wants the saved row; stop re-applying it. + pendingScrollRestoreRef.current = null; + }, []); + const syncBottomStateFromScroll = useCallback(() => { const scrollArea = scrollAreaRef.current; if (!scrollArea) return; @@ -733,13 +746,7 @@ export function BottomAnchoredScrollBody({ } if (nearBottom) { - userDetachedFromBottomRef.current = false; - shouldStickToBottomRef.current = true; - userScrollIntentUntilRef.current = 0; - setIsAtBottom(true); - // A deliberate scroll to the bottom during the restore settle window means - // the user no longer wants the saved row; stop re-applying it. - pendingScrollRestoreRef.current = null; + attachToBottom(); return; } @@ -752,6 +759,7 @@ export function BottomAnchoredScrollBody({ // The user is scrolling on their own; don't yank them back to the anchor. pendingScrollRestoreRef.current = null; }, [ + attachToBottom, cancelQueuedRestore, hasRecentUserScrollIntent, readMaxScrollOffset, @@ -799,20 +807,46 @@ export function BottomAnchoredScrollBody({ const handleScrollAreaResize = useCallback(() => { const scrollArea = scrollAreaRef.current; + let shrankOntoBottomWhileDetached = false; if (scrollArea) { // The steady-state cache refresh: the observer watches both the scroll // port and the content wrapper, so every legitimate // scrollHeight/clientHeight change passes through here. The first // delivery is also what makes the cache authoritative for hot-path // reads (see resizeObserverHasDeliveredRef). - refreshMaxScrollOffset(scrollArea); + const previousMaxScrollOffset = maxScrollOffsetRef.current; + const cacheWasAuthoritative = resizeObserverHasDeliveredRef.current; + const maxScrollOffset = refreshMaxScrollOffset(scrollArea); resizeObserverHasDeliveredRef.current = true; + shrankOntoBottomWhileDetached = + cacheWasAuthoritative && + !shouldStickToBottomRef.current && + maxScrollOffset < previousMaxScrollOffset && + isScrolledNearBottom(maxScrollOffset, scrollArea.scrollTop); } // While a restore is pending, the ResizeObserver is the settle signal; the // bottom-restore is suppressed (stick-to-bottom is false) anyway. if (advancePendingScrollRestore()) return; + if (shrankOntoBottomWhileDetached && scrollArea) { + // The detached mirror of the attach->detach edge in + // syncBottomStateFromScroll: a content shrink (collapsing a long tool + // output near the end) clamped a detached viewport onto the new, + // smaller maximum. The browser delivered that clamp's scroll event + // before this refresh, so the scroll handler classified it against the + // stale, larger cache and left the viewport detached. A live read used + // to re-attach on that very scroll event; do the same here, against + // fresh geometry, so streaming content keeps following the bottom. + attachToBottom(); + writeScrollAnchor(scrollArea); + } queueBottomRestore(); - }, [advancePendingScrollRestore, queueBottomRestore, refreshMaxScrollOffset]); + }, [ + advancePendingScrollRestore, + attachToBottom, + queueBottomRestore, + refreshMaxScrollOffset, + writeScrollAnchor, + ]); // Begin restoring the saved scroll position on mount, before the listener // effect's `queueBottomRestore()` runs (a useEffect, which runs after layout