Skip to content

Let taps paint: transition-priority navigation, deferred expanders and sidebar realization - #2386

Closed
vburojevic wants to merge 4 commits into
get-bb:mainfrom
vburojevic:bb/mobile-perf2/interaction-yielding
Closed

Let taps paint: transition-priority navigation, deferred expanders and sidebar realization#2386
vburojevic wants to merge 4 commits into
get-bb:mainfrom
vburojevic:bb/mobile-perf2/interaction-yielding

Conversation

@vburojevic

Copy link
Copy Markdown
Contributor

What was wrong

Interaction taps on mobile web ran their entire React consequence as one discrete synchronous commit, so the browser could not paint tap feedback first. The perf-watch ledger attributes 671s/267 stalls over 7 days to click on button.inline-flex (the ExpandablePanel header and shared Button class) and 858s/364 stalls to after navigate: the expander body materialized inside the click's render, openMobileSidebar flushed the whole ProjectList/ThreadRow subtree before the slide's first frame, navigation exposed no pending signal, and the split-pane maximize toggle rewrote scrollLeft/scrollTop on every tracked element for 30 unconditional rAF frames.

What changed

  • app-route-anchor.tsx: navigateRoute wraps navigation in useTransition's startTransition; the navigate context identity is unchanged, and isPending is exposed through a separate RouteNavigationPendingContext + useIsRouteNavigationPending() so navigate consumers don't re-render per navigation. RootComposeMobileRecents.tsx swaps its raw <Link> for RouteAnchor. (react-router v7 already transitions router state; the delta is the pending signal plus the remaining urgent work riding the same transition.)
  • disclosure.tsx: the expander body realizes off useDeferredValue(isExpanded) — caret/header stay urgent, the body mounts in a follow-up interruptible commit; closing-body retention and the layout-animation signal unchanged.
  • sidebar.tsx: realizeMobileSidebar() runs inside React.startTransition; drag-style write order unchanged; the settle commit's render-phase latch still covers the race. The file's flushSync sites were reviewed individually and deliberately left alone.
  • SplitThreadArea.tsx: restore() compares before writing and returns whether anything moved; the loop stops after the first zero-correction frame, capped at 5 frames.

How you verified

  • New tests, each proven fail-before/pass-after: transition navigation samples {isPending, old pathname} in the tap's commit; the deferred body is absent from the urgent flush and lands in the deferred commit (plus a closing-body retention guard); the sidebar realize commit is absent from the tap's flush; settled restore performs 0 scroll writes (before: 31), an adversarial scroller is corrected ≤6 times.
  • pnpm exec turbo run typecheck --filter=@bb/app exit 0; focused suites 67 files / 620 tests pass; lint 0 errors.
  • Merged-tree WebKit QA (iPhone viewport): section expand, sidebar open/close, thread navigation — all pass with zero console errors.

AGENT GENERATED

vburojevic and others added 4 commits August 24, 2026 09:15
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>
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>
@SawyerHood

Copy link
Copy Markdown
Collaborator

Closing in favor of #2435, which integrates this PR on current main together with #2385, #2386, #2388, #2389 and #2392 and the fixes from the adversarial review. Your commits are cherry-picked unchanged with you as author, and every fix commit carries a Co-authored-by trailer for you, so credit follows the work whichever merge method is used. The branch stays as-is.

Fix landed there for this PR:

  • In disclosure.tsx the region's content branch, height sync, transition classes, deadline and in-flight accounting now 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.
  • Left as-is: the transition-priority navigation is a no-op because react-router already transitions; the deferred-body starvation under heavy streaming is an open trade-off noted in Mobile perf: integrate #2385, #2386, #2388, #2389, #2392 with the review fixes #2435.

Thanks for the work: the design held up under review; only the items above needed changing.

AGENT GENERATED

@SawyerHood SawyerHood closed this Aug 25, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants