[stacked on #2385 + #2386] Consolidate per-row ResizeObservers into shared read/write-phased observers - #2392
Closed
vburojevic wants to merge 12 commits into
Closed
Conversation
Every route tap ran navigateRef.current() bare inside the click's discrete event. Wrap it in useTransition's startTransition inside RouteNavigationProvider so the tap's urgent commit paints first, and expose isPending through a separate RouteNavigationPendingContext (the navigate context identity stays stable, so navigate consumers still never re-render per navigation). Replace the raw react-router <Link> in RootComposeMobileRecents with RouteAnchor so the mobile recents rows take the same path. New test proves ordering: the tap's commit shows pending with the old route still mounted, and the destination lands in a later transition commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Expanding a collapsed ExpandablePanel materialized the whole body subtree inside the click's discrete commit (button.inline-flex stalls in the hang ledger). Drive the expandedBody memo from useDeferredValue(isExpanded) so the caret/header flip paints in the tap's first frame and the body mounts in a follow-up interruptible commit. Header state, the closing-body ref retention, and the layout-animation signal stay on the urgent value; rows that mount already expanded still render their body immediately (useDeferredValue returns the live value on first render). New test fails before this change: with flushSync standing in for the tap's urgent flush, the body used to be mounted in that same commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
On every iOS visual-viewport tick (keyboard animation, URL-bar collapse, momentum settling) the shell hook forced a full-document layout via document.body.clientHeight and rewrote shell top/height plus the inherited --bb-shell-height, invalidating computed style for the whole app tree at animation cadence. - Skip all style writes when a pass recomputes the geometry it already applied, and make clearViewportOverride a no-op while nothing is set. - Cache the containing-block height; re-read it only on triggers that can resize the layout viewport (window resize, orientationchange, focusin) — never on visualViewport ticks, which move only the visual viewport. - Gate visualViewport scroll ticks on keyboard focus or an applied override: keyboard-less URL-bar pans need no compensation, while embedded-browser overrides (applied without a keyboard) keep tracking. The focusout fast-restore, the native-layout early-exit, the pinch-zoom guard, and rAF coalescing are preserved; programmatic focus (composer autofocus) still triggers one freshly measured pass, covered by a new test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When boot's idle pre-realization has not run yet, the sidebar trigger's click flushed the whole ProjectList/ThreadRow subtree synchronously before the slide's first frame could composite. Wrap realizeMobileSidebar() in React.startTransition so the tap's flush only writes the inline drag styles (the slide starts immediately) and the subtree mounts interruptibly during the settle window. The drag-style write order is unchanged, and the settle commit's render-phase latch still realizes the subtree synchronously if it somehow lands first. The other flushSync sites in this file run after the settle window (deferred open/close commits and the swipe settle paths), not in the tap's critical path, and are deliberately untouched. New test fails before this change: with flushSync standing in for the tap's urgent flush, the subtree used to be realized in that same flush. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The maximize/restore scroll-preservation loop unconditionally rewrote scrollLeft/scrollTop on every tracked element for 30 animation frames, forcing layout each frame for half a second after every toggle. Make restore() compare before writing and report whether anything needed correction, stop the rAF loop after the first frame with zero corrections, and cap the loop at 5 frames. The pre-paint initial restore() stays. New test fails before this change: the settled case saw 31 scroll writes (pre-paint + 30 frames); now it sees none, and an adversarial scroller that keeps normalizing to zero is corrected at most 6 times. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every timeline size change re-entered layout up to five times: the scroll body's ResizeObserver delivery did a live scrollHeight/clientHeight refresh, the observed frame's bottom restore read again, and each of the three rAF settle-tail frames forced another layout — at streaming cadence on an unwindowed tree. - bottom-anchored-scroll-body: refresh the cached max offset from the ResizeObserver's own box sizes when the delivery carries entries (the scroll port's content box + the content wrapper's border box), falling back to the live read for entry-less deliveries (test stubs). The observed frame's restoreBottomOnce keeps its deliberate live read; the settle-tail frames now reuse the cache, and a tail frame that corrected drift arms exactly one live verification read on the next frame. - height-transition: size the wrapper from the entry's borderBoxSize (the same border-box metric as the offsetHeight used by the mount and snap paths) instead of the content rect; non-observer paths keep offsetHeight. The scroll-preservation contract suite passes unmodified. New settle-tail tests count geometry reads per tail frame and cover the entry-derived cache; a height-transition test pins the border-box sizing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every scroll event wrote data-scrollbar-scrolling (matched only by desktop ::-webkit-scrollbar rules) — a pure style invalidation on touch — and each throttled scroll-anchor sample re-ran querySelectorAll over the scroll subtree at 10 Hz while the browser was busy scrolling. - Skip the transient-scrollbar attribute when (pointer: coarse) matches. - Cache the scroll-anchor row NodeList in a ref; the existing ResizeObserver invalidates it, and an end-connectivity check covers windowed row swaps that keep the content size constant. - Raise the scroll-anchor capture throttle to 250ms on coarse pointers — restore-on-return needs the resting position (always carried by the trailing write), not mid-flick samples. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The previous comment credited Android's resizes-content path, but the focusin listener only exists on iOS WebKit. The real reason: the pass that sizes the shell for the arriving keyboard must start from the real containing block, and focus changes are rare enough to afford the read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rf2/observer-consolidation
Every ExpandablePanel and HeightTransition/AutoHeightContainer installed its own ResizeObserver whose callback interleaved a layout read with a style write, so one width/height event (iOS keyboard, drawer, orientation, font swap) forced a synchronous layout pass per mounted row. The new src/lib/shared-resize-observer.ts registry runs on a single module-level observer and dispatches each batch in phases — every registration's read completes before any write — bounding a whole batch at one forced layout. Panel body sizing now comes from the entry's border box (offsetHeight's metric, no layout read), preserving the transitionDuration snap semantics, the deferred-body realization, and the borderBoxSize sizing the existing suites pin. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every message mounted two width observers: one for the action row's slot and one for the enclosing [data-message-column], even though the mobile overflow branch renders a constant layout that never reads the slot width, and the column width is the same number for every top-level row. useMeasuredWidth gains an `enabled` option (hook order stable, no observer constructed when disabled); the overflow branch disables the slot observer; and the top-level TimelineRowsList measures its root once and shares it through MessageColumnWidthContext, so one observer serves every bar. Without a provider (stories, unit renders) or inside nested, narrower lists — which shadow the context with null — a bar measures its own column exactly as before. Desktop inline/overflow layout is pinned by the existing width-driven tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The shared document pointerdown/pointerup/pointercancel handlers in
SelectableMessageProse never call preventDefault, but without the
passive flag the browser must still treat every tap as potentially
blocking. Declare { passive: true } on the three pointer listeners and
pin the flag with a test; removal matching is unaffected (only the
capture flag participates), so the shared teardown behavior is
unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
4 tasks
This was referenced Aug 25, 2026
Closed
Collaborator
|
Closing in favor of #2435, which integrates this PR on current Fix landed there for this PR:
Thanks for the work: the design held up under review; only the items above needed changing.
|
SawyerHood
added a commit
that referenced
this pull request
Aug 25, 2026
…iew fixes (#2435) ## Human comments ## What was wrong Five of the mobile-perf2 PRs by @vburojevic (#2385, #2386, #2388, #2389, #2392) were each sound in design but carried one or two confirmed defects that an adversarial review found: the touch scrollbar thumb never showed after the per-scroll trim, the cached scroll-anchor row list went stale on windowed timelines, the deferred expander body blanked its preview and animated toward an empty region, the host-disconnect status fan-out ran ~5–7 synchronous queries for every thread on the host, a status change that raced an in-flight search was never refetched, the `max-age=300` app shell let browsers and the Electron window boot a stale shell whose hashed assets 404 for up to five minutes after an update, and the shared action-bar width was 16 px wider than the assistant column. This PR integrates the five on current `main` and fixes each finding, so the set can land together instead of one at a time. Credit: every original commit keeps @vburojevic as author, and every fix commit carries a `Co-authored-by` trailer for them. A squash merge credits them through those trailers; a merge or rebase keeps the authorship as-is. ## What changed Original work (21 commits, cherry-picked in PR order, unchanged): - #2385 Calm the iOS shell-geometry handler and the timeline resize cascade. - #2386 Let taps paint: transition-priority navigation, deferred expanders and sidebar realization. - #2392 Consolidate per-row ResizeObservers into shared read/write-phased observers. - #2388 Realtime round 2: status-change metadata everywhere, reconnect gating, coalesced fallback refetches. - #2389 Boot and delivery quick wins for the relayed mobile path. Fixes (13 commits; each names the finding it closes): - `bottom-anchored-scroll-body.tsx`: the `data-scrollbar-scrolling` write is back on every pointer, idempotent (one write per scroll burst); `.thread-scrollbar`/`.transient-scrollbar` are not pointer-gated, so skipping it on touch hid the thumb. The scroll-anchor row cache is bypassed when the top-level list holds a `[data-timeline-virtual-spacer]`; unwindowed timelines keep it. The scroll-preservation suite now exercises the entries-derived resize path and the scroll-gate test pins the keyboard-pan compensation. - `disclosure.tsx`: the region's content branch, height sync, transition classes, deadline and in-flight accounting key on the deferred expanded value, so the collapsed preview stays until the body exists and the tween starts from the real body; re-expanding inside the 200 ms close window keeps the retained body. - `session-owner-side-effects.ts` + `packages/db` (`listActiveHostThreads`): daemon close, disconnect grace and host removal build `statusChange` metadata only for active threads, fetched in batched queries; idle threads keep the pre-PR bare push. - `realtime-cache-registry.ts`: the `statusChange` search invalidation keeps `cancelRefetch: false` and schedules one trailing refetch after the in-flight search settles; the same two-line gap in the completed-turn path is fixed in its own commit. The coarse-pointer debounce test moved to its own file so the 65-test suite runs in the shared vitest worker again. - `server.ts`: the app shell is served `Cache-Control: no-cache` + weak build-id ETag again; the If-None-Match → 304 path is unchanged. `apps/connect/src/cache.ts`: the worker's revalidated-shell contract is now `no-cache` + ETag; the edge copy is stored with an internal 300 s bound, served only after the origin's 304, and the visitor always receives the origin's `no-cache`. `no-store`/`private`/set-cookie still bypass; the plain asset path still rejects `no-cache`. Known limit (unchanged from #2389): the edge document copy is rarely served in the mobile flow because browsers keep their own copy. - `document-cache.test.ts`: each test stores its own edge copy. `bundle-budget.json`: `maxBootBrotliBytes` 479,067 → 429,072 (10% above the measured boot payload); `maxBootBytes` unchanged. `vite-font-preload.test.ts`: head order pinned against a synthetic document, since the dist-gated suite is skipped in CI. - `MessageActionBar.tsx` / `ThreadTimelineRows.tsx` / `ConversationMessageContent.tsx`: the shared list width subtracts the assistant column's `px-2` inset; the class and the 16 px constant are declared together. No wire change between server and host daemon (`HOST_DAEMON_PROTOCOL_VERSION` unchanged; #2388's one-line `daemon-protocol.ts` edit adds no field). No CLI, guide, or doc surface changes. Not addressed, by decision: a deferred expander body can lag behind heavy streaming updates until React's transition expiry (a timeout fallback changes when the expensive render blocks the main thread, which is the trade-off #2386 is about); the transition-priority navigation in #2386 is a no-op because react-router already transitions (harmless, left as-is). ## How you verified Review: two independent multi-agent passes over each PR (correctness, claims audit with the PR's tests run, repo-rule/contract audit, one PR-specific lens), every finding checked by three refuters (code trace, a throwaway experiment against the real code, an impact judge); only findings that survived at least two of three were fixed. Fixes: each fix commit came with a test proven to fail before and pass after, and passed two independent verifiers (a diff reviewer and a runner that re-proved fail-before by restoring the pre-fix sources and ran the package suite) in one round. Final branch (`3388bcb45`): - `pnpm exec turbo run typecheck --filter=@bb/app --filter=@bb/server --filter=@bb/connect --filter=@bb/db --filter=@bb/desktop --filter=@bb/mobile --filter=@bb/cli --filter=@bb/sdk --continue` — exit 0. - `pnpm exec turbo run lint --filter=@bb/app --filter=@bb/server --filter=@bb/connect --continue` — 0 errors. - Full suites: `@bb/db` 409/409, `@bb/connect` 120/120 (the document-cache suite runs the real worker in workerd: cold store, 304-only repeat, new build on next navigation, visitor 304 relayed, pre-contract server proxied uncached), `@bb/server` 2,047/2,047, `@bb/app` 3,404 passed / 4 skipped / 0 failed. - `pnpm exec turbo run build --filter=@bb/app` + `node apps/app/scripts/check-bundle-budget.mjs` — OK: boot 1,547.5 KB raw / 381.1 KB brotli, 3 boot chunks. - EAP codename scan: clean for the working tree, tracked files, and the added lines and commit messages of `origin/main..HEAD`. Still to do by hand before merge: a physical iPhone and Android pass for #2385/#2392 (keyboard open/close, URL-bar collapse, rotation with many expanded rows); the CI iOS-simulator job is skipped. The `@bb/mobile`, `@bb/cli`, `@bb/sdk`, `@bb/integration-tests` suites and the packaged tarball smoke were green on an earlier nine-PR merge tree, not yet re-run on this exact branch. Supersedes #2385, #2386, #2388, #2389, #2392. > AGENT GENERATED --------- Co-authored-by: Vedran Burojevic <vedran.burojevic@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Sawyer Hood <kirbyhood@gmail.com>
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.
STACKED PR — contains #2385 (ios-shell-geometry) and #2386 (interaction-yielding); only the top 3 commits are new. Do not merge before both bases.
What was wrong
Every collapsible timeline row installed its own ResizeObserver whose callback interleaved a layout read (
offsetHeight) with a style write, and every message mounted two more observers for its action bar — one never read on the mobile-overflow path, the other measuring a column width identical for every row. When one event resizes the timeline (iOS keyboard, drawer, orientation,--bb-shell-height), WebKit delivers all observers in one batch and the interleaved pattern defeats batching: N rows ⇒ N synchronous layout passes. The shared selection pointer listeners were also registered non-passive despite never callingpreventDefault.What changed
apps/app/src/lib/shared-resize-observer.ts: one module-level observer with registered{read(entry), write(value)}phases — all reads complete before any write, bounding a batch at one forced layout.ExpandablePanel,HeightTransition, andAutoHeightContainermigrated onto it; panel sizing comes from the entry's border box (no layout read), preserving thetransitionDurationsnap, the deferred-body realization (Let taps paint: transition-priority navigation, deferred expanders and sidebar realization #2386), and the borderBoxSize sizing (Calm the iOS shell-geometry handler and the timeline resize cascade #2385).useMeasuredWidthgainsenabled(observer skipped, hook order stable); the mobile-overflow bar branch drops its slot observer; the top-levelTimelineRowsListmeasures the message-column width once and shares it viaMessageColumnWidthContext(nested lists shadow withnulland self-measure as before). Per bar: 2 observers → ≤1 desktop, 0 mobile-overflow.{ passive: true }on SelectableMessageProse's shared pointer listeners (handlers verified free ofpreventDefault).Merge-order note vs #2391 (timeline windowing): both touch
ThreadTimelineRows.tsxin one JSX hunk. Verified resolution (used on the integration branch): take this PR's structure verbatim (provider wrapper +messageColumnWidthSourceRef+ re-indentation) and inside it setminItemCount={spacing === "top-level" ? (isCompactViewport ? 16 : 60) : 20}from #2391. Whichever merges second applies exactly that.How you verified
length of 1 but got 2before), action-bar observer-budget + list-level wiring (expected 2 to be 1before), passive-flag pin (optionsundefinedbefore).