Land the mobile-performance batch (#2277-#2281) and fix a scroll re-attach edge - #2300
Merged
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…ffer 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 <noreply@anthropic.com>
… the debounce buffer
…nto 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 <vedran.burojevic@gmail.com> Co-Authored-By: Claude <noreply@anthropic.com>
This was referenced Aug 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Integration of five mobile-performance PRs by @vburojevic (#2277, #2278, #2279, #2280, #2281) plus one follow-up fix found while reviewing them for regressions.
Authorship note: this lands as a squash because merge commits are disabled on this repo. The five branches are @vburojevic's work — co-author trailers on the squash preserve credit, and the source PRs are linked below.
What was wrong
Five independent mobile-performance defects, each with its own root cause:
SecondaryPanelLayoutPanelGroup(its group key defaulted toresetKey= thread id), so the realized secondary panel was destroyed and rebuilt on every navigate and panel sizes reset.[events-appended, queue-changed, status-changed]as one message, so mid-stream coalescing collapsed exactly when it mattered, and one thread's status flip flushed every other streaming thread's buffer.scrollHeight/clientHeight(forced layout in WebKit) on an unvirtualized list of thousands of nodes.ThreadDetailPromptArearebuilt its publishedPluginComposerHoston every keystroke, so the identity change re-rendered the whole thread shell per character. Production telemetry attributed the worst mobile hang (19.6 s after keydown) to this.Follow-up fix (this branch's own commit): #2280 guards the attach→detach edge with one fresh read, but not its mirror. A detached viewport that a content shrink clamps onto the new, smaller maximum (collapsing a long tool output near the end while scrolled up) is delivered its clamp scroll event before the
ResizeObserverrefresh, so the handler classifies it against the stale, larger cache and leaves it detached — with no later re-test. Before the cache, the live read re-attached on that very event. User-visible result: the jump-to-bottom affordance stays up while already at the bottom, and streaming content stops following.What changed
Per source PR, unchanged from the reviewed branches:
panelGroupKeyescape hatch so the physical host survives navigation while content identity resets viaresetKey; thehasPanelExpandedRefmount-collapse guard re-arms per thread.createAppQueryClientgains an injectableshouldRefetchOnWindowFocus;maingates it onwsManager.getConnectionState() !== "connected". Per-query policies still win.maxScrollOffsetin a ref refreshed only where layout legitimately changes; hot handlers readscrollTopplus the cache.PluginComposerHostdrops itsdraftfield for an identity-stablegetCurrent()+subscribeDraft()contract, withusePluginComposerHostDraftas the only reactive read.New in this branch —
apps/app/src/components/ui/bottom-anchored-scroll-body.tsx: detect the clamp case in theResizeObserverpath (cache authoritative, max offset shrank, viewport detached, now inside the bottom threshold), re-attach through a sharedattachToBottomextracted from the scroll handler, and write the at-bottom anchor. No new reads in the scroll hot path. A shrink that does not reach the viewport leaves it alone.No wire changes: nothing here touches the server/daemon protocol, so
HOST_DAEMON_PROTOCOL_VERSIONis unchanged.PluginComposerHostis internal toapps/app— the plugin SDK surface is untouched, anduseComposerView/useComposerreturn identical values.How you verified
origin/main; all five PRs merged with no conflicts (includingThreadDetailSecondaryContent.test.tsx, touched by both Keep the thread panel host mounted across thread navigation #2277 and Make the plugin composer host pull-based so keystrokes stop re-rendering the thread shell #2281).pnpm exec turbo run typecheck --filter=@bb/app— pass.pnpm exec turbo run lint --filter=@bb/app— 0 errors. The touched files carry no lint warnings onmainor here (the 181 warnings are pre-existing elsewhere; the oneset-state-in-effectwarning in the scroll body also exists onmain).pnpm exec turbo run test --filter=@bb/app --force— 422 files, 3,288 passed, 3 skipped.re-attaches a detached viewport that a content shrink clamps onto the bottomfails on the Stop forced layout reads in the bottom-anchored scroll hot path #2280 head and passes here; a second test (leaves a detached viewport alone when content shrinks without reaching it) guards the inverse.queued-messages.ts:390,thread-lifecycle.ts:1562-1568/1669-1682), the resizable-panels callback semantics, and the draft-store/ref-sync ordering.Known trade accepted by the owner, disclosed in #2278: while the socket reports connected, queries with neither realtime coverage nor a focus policy (Skills, plugin settings/contributions/catalog, host-path, changelog, plugin SDK
useSettings) skip the focus refresh and repair on remount. A half-open socket delays realtime catch-up by up to the 5 s pong timeout.Not verified: no iOS device/simulator run. The scroll fix relies on the same clamp-before-observer ordering #2280 specifies.
Supersedes #2277, #2278, #2279, #2280, #2281 (closed on merge; this squash carries their changes).
No issue to close: these PRs were opened directly, so the template's
Fixes #line is intentionally omitted.