Mobile perf: integrate #2385, #2386, #2388, #2389, #2392 with the review fixes - #2435
Merged
Conversation
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> (cherry picked from commit e0c2d3c)
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> (cherry picked from commit e4dc82b)
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> (cherry picked from commit 22f41ab)
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> (cherry picked from commit d1d20fe)
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> (cherry picked from commit 2ca2f13)
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> (cherry picked from commit 61b765b)
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> (cherry picked from commit 7b22677)
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> (cherry picked from commit 28f6f59)
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> (cherry picked from commit 6472681)
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> (cherry picked from commit b1b2497)
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>
(cherry picked from commit e1d7412)
The host connectivity fan-out (daemon socket close, disconnect grace, host removal) and the post-commit interruption publish sent bare status-changed notifications, so every client fell back to refetching every active thread list once per thread on the host. Build the row snapshot with the existing buildThreadStatusChangeMetadata builder at the sites that run outside a transaction with a real hub, and mark the in-transaction sites (buffered DbNotifier, no runtime hub or provider registry) as deliberately bare — the client-side throttled fallback covers those. thread-send.ts, parent-system-messages.ts and queued-messages.ts have attached the builder since #2169; the queued auto-send path gains a guard test modeled on the lifecycle-outcome coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 07f8adf)
A status-changed push without a row snapshot falls back to refetching every active thread list plus the sidebar bootstrap on the immediate path. Bare pushes arrive in bursts (in-transaction writers publish one per thread), so route the fallback through the existing throttled active-refetch machinery: everything still goes stale immediately, the first push refetches right away, later pushes inside the second coalesce into one trailing refetch, and no fetch in flight is cancelled. Archived pages keep their stale-only treatment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 53da3a5)
The metadata branch of patchThreadListStatusState returned the search prefix for the generic handler loop to invalidate, which uses the default cancelRefetch and aborts a search request already in flight. Status changes ride the un-debounced immediate path, so on a slow link a streaming thread's flips could re-issue (and starve) an open search indefinitely — the same starvation the debounced path already fixed for turn completion. Invalidate the prefix directly with cancelRefetch: false, once per flush. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 615f16a)
createAppQueryClient gated the focus refetch (#2278) but left refetchOnReconnect at TanStack's default true, so every browser `online` event — which mobile Safari re-fires around the same suspensions the focus gate absorbs — refetched every active query while the socket and reconnect watermark already owned freshness. Apply the same lost-coverage gate to reconnect. The two policies with explicit refetchOnReconnect: true keep it deliberately: provider usage limits have no realtime change kind, and the resume-refetch surfaces (thread tabs, host file preview) opt out of the gate by design. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 4dfd6cd)
LiveDurationText owned a setInterval per in-flight timeline row while the shared useSecondTick ticker existed for exactly this; every badge now rides one interval, and that interval stops entirely while the document is hidden, with an immediate tick on resume so durations jump to current truth. The realtime thread-invalidation debounce also reads the pointer class once at module init: coarse-pointer (touch) devices widen 50/200 to 150/400 so streaming reconciles stop competing with scrolling on a phone core, while desktop keeps the existing cadence. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 691dd02)
A cold isolate's page-load burst (100+ authenticated requests) paid one label-resolve and one session-verify D1 round trip per request until the first one settled, because labelCache/sessionCache stored only settled values. Store the in-flight promise at query start instead, so request 2..N of a burst join request 1's round trip; a rejected lookup is evicted on settle so a D1 hiccup cannot poison a key for its TTL. TTLs and the fresh-read bypass are unchanged (they encode revocation latency). New tests fail before (6 D1 selects for a 6-request burst) and pass after (1 select), for both resolveLabel and verifySessionCookie. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit c11a1cc)
…n 007 step 1) Vite appends its built asset tags as [entry script, 69 modulepreloads, stylesheet], and the font preload plugin appended after that, so the render-blocking stylesheet was the 68th resource the preload scanner discovered and the Inter preload was dead last. The connect tunnel serializes responses FIFO on one WebSocket, so discovery order is delivery order: first paint sat behind ~1.5 MB of JavaScript. The bb:font-preload post transform now performs the head surgery itself: it moves the stylesheet (with fetchpriority=high) and the font preload ahead of the entry script and modulepreload block, keeps the pre-paint theme script ahead of the stylesheet (build fails loudly if that ever inverts), and leaves the body palette script's append-last contract intact. The new emitted-order test asserts against the real dist/index.html: before this change it failed with stylesheet at byte 9654 vs first modulepreload at 3861; after, the order is theme script (2891) < font preload (3801) < stylesheet (3922) < entry (3985) < modulepreloads (4065). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit cb6c05e)
Rolldown's automatic splitting left the boot payload as 70 chunks, 34 of them under 4 KB and 18 under the 1 KiB precompress floor — half the boot requests carried ~2% of the bytes, and on the relayed mobile path each one is a full worker → DO → tunnel → laptop round trip. Two advancedChunks groups tagged $initial (the entry's static-import closure) merge that graph: a vendor group so app-only releases keep the vendor hash cacheable, and an app group for the rest. Lazy-route and on-demand facades are untouched (their modules are not $initial), so the budget's closure walk, forbiddenPackages and onDemandPackages gates hold unchanged. Measured (bundle-stats.json + check-bundle-budget): boot chunks 70 -> 3 boot raw 1575.8 KB -> 1548.2 KB boot brotli 443.0 KB -> 381.2 KB SplitWorkspaceRoute closure 2018.4/538.6 KB -> 2001.1/533.5 KB (45 chunks) index.html 10.8 KB -> 5.2 KB (69 -> 2 modulepreloads) bundle budget OK Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 0203c0b)
…lan 007 step 3) .html was missing from COMPRESSIBLE_EXTENSIONS, so the app shell had no .br/.gz sidecar and every cold navigation shipped it re-compressed on the fly at gzip stream quality through the tunnel. The SPA fallback (the document response for every client route a phone opens) also bypassed findPrecompressedStaticFile entirely, reading index.html as utf8. Add .html to the precompress set (index.html.br: 5.2 KB -> 1.6 KB) and route the fallback through the same sidecar-aware serving path as a direct file hit. text/html already passes the precompressed content-type allowlist, so a direct /index.html hit picks the sidecar up unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 06c0ebd)
…007 step 4) The shell was no-cache, which apps/connect/src/cache.ts treats as never-cacheable: every cold navigation paid a full worker -> DO -> tunnel -> laptop round trip for the document before the browser learned what to fetch. no-cache existed so a new build is picked up immediately — that property had to survive. Server half: the shell (direct and SPA-fallback, unified behind registerStaticAppRoutes) now carries max-age=300, must-revalidate plus a weak build-id ETag derived from the served file's bytes (index.html embeds every hashed asset URL, so the content hash IS the build id; cached per path, revalidated by size+mtime). If-None-Match answers with an empty 304 carrying the same validator and cache-control. Connect half: serveWithCache learns a revalidated-shell flavor, checked before plain cacheability (the shell's max-age=300 would otherwise be cached without the revalidation its must-revalidate demands). The last confirmed document is stored in caches.default with its origin headers intact — its own max-age bounds storage at <=300s — and every navigation revalidates through the tunnel: the visitor's If-None-Match is forwarded when present (304 relayed), otherwise the stored ETag makes the round trip a 304 and the body is served from the edge, rebuilt pre-encoded exactly like an asset hit. A fresh 200 replaces the stored copy, so a new build takes effect on the next navigation; an origin that stops speaking the contract (dev server) gets its stored copy dropped. Deviation from the plan sketch: one self-describing cache entry (ETag in the stored response's own header, confirmed by the origin before every serve) instead of separate (label, ETag)-keyed body + pointer entries. Equivalent consistency, and it keeps cache.put on the proven clone-of-subrequest path — workerd's put of a header-rewritten rebuild has exactly the encoding ambiguity response-encoding.ts exists to avoid. Old worker + new server skew is safe (the old worker plain-caches the shell for at most 300s); new worker + old server is inert (no ETag, no must-revalidate -> no shell flow). No server<->host-daemon wire change, so no HOST_DAEMON_PROTOCOL_VERSION bump. Verified: new apps/server/src/static-shell.test.ts (sidecar + ETag + 304 on both paths, ETag rotation on a new build); static-cache.test.ts updated from the old no-cache pin; new apps/connect/src/ document-cache.test.ts in real workerd via the tunnel fixture — repeat navigation served from caches.default with only a 304 on the tunnel, build change shipped on the next navigation, visitor 304 relayed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit f70f363)
PR #2385 stopped writing data-scrollbar-scrolling on coarse pointers on the assumption that the attribute only feeds desktop ::-webkit-scrollbar rules. The .thread-scrollbar/.transient-scrollbar rules in app.css are not pointer-gated and set `scrollbar-color: transparent transparent` at rest, and Android Chrome and iOS apply scrollbar-color to their overlay indicators, so a touch flick never showed a thumb at all. Write the attribute on every pointer again, but only when it is not already set: the thumb toggles once per scroll burst (plus the 600ms idle clear) instead of re-invalidating style on every scroll event. The coarse-pointer test now asserts one attribute write per burst through a MutationObserver, and the cadence test settles its first capture by advancing the clock rather than running pending timers, which would now also fire the idle timeout and move the faked clock past the write. Co-authored-by: Vedran Burojevic <vedran.burojevic@gmail.com> Co-Authored-By: Claude <noreply@anthropic.com>
The row NodeList cache from PR #2385 was invalidated only by a ResizeObserver delivery or by its first/last row disconnecting. On the TanStack-windowed timeline neither happens for a window slide: rows are absolutely positioned inside a spacer whose height does not change when already-measured rows swap, and the last row is force-mounted (alwaysMountedKeys), so the stale list was reused and getTopMostVisibleRow treated the unmounted rows' empty rects as "above", recording the always-mounted last row (or the old window's first row) as the anchor. The throttled captures, the trailing write and the unmount flush all persisted it, so returning to the thread restored the wrong row. Report from getScrollAnchorRows whether the top-level list holds a [data-timeline-virtual-spacer] and skip the cache in that case: only the rows near the viewport are mounted there, so the query is cheap, while the unwindowed timeline keeps the cache. The scroll-preservation suite now mounts a spacer, slides the window without a resize delivery and checks that both the next capture and the unmount flush follow the new top-most row; its row-rect mock reports an empty rect for a disconnected row, as a browser does. Co-authored-by: Vedran Burojevic <vedran.burojevic@gmail.com> Co-Authored-By: Claude <noreply@anthropic.com>
The scroll-preservation suite's ResizeObserver stub delivered no entries, so every #2280/#2300 contract case ran the entry-less live-read fallback rather than the entry-derived cache refresh that every browser takes since PR #2385; the one settle-tail case on that path used identical content and border boxes, so swapping the two box reads went unnoticed. The stub now delivers an entry per observed target with the box sizes a browser reports (scroll port content box = client height, content wrapper border box = scroll height) and the other box of each pair offset by 8px, and the settle-tail case uses distinct boxes plus a stays-detached step that a wrong box read would re-attach. The keyboard half of the visual-viewport scroll-gate test passed even with the keyboard branch of handleVisualViewportScroll removed, because focusing the editor already scheduled the compensating pass. It now flushes the focus pass with no pan, then delivers the pan as a scroll-only tick, so the 340px compensation can only come from that branch. Co-authored-by: Vedran Burojevic <vedran.burojevic@gmail.com> Co-Authored-By: Claude <noreply@anthropic.com>
The body of ExpandablePanel realizes off useDeferredValue(isExpanded), but the region around it (AnimatedExpandablePanelContent's content branch and height sync, the toggle deadline, the grid/opacity classes and the layoutAnimationInFlightCount window) still keyed on the urgent isExpanded. A tap's commit therefore dropped the collapsed preview for an empty wrapper, armed the 200ms height tween toward that wrapper and opened the in-flight window; the body then landed a commit later, re-targeting the tween mid-flight or, past the deadline, snapping in with transitionDuration 0s. Reopening inside the 200ms close window also rendered the still-null deferred body in the tap's commit, so the retained subtree unmounted and a fresh one mounted a commit later. Derive isBodyExpanded = isExpanded && (deferredIsExpanded || isClosing) and key every region concern on it, so the region opens in the commit that mounts the body (the deferred one, or the reopen tap's own while the close window retains the subtree) and closes in the collapse tap's commit. The caret and aria-expanded stay on the urgent value. The rendered body follows the deferred value, with the retained ref as the fallback, and the ref sync is gated on the deferred value so the tap-to-body gap can no longer clear it. As a consequence a collapse now animates out from the same element instead of remounting it. Co-authored-by: Vedran Burojevic <vedran.burojevic@gmail.com> Co-Authored-By: Claude <noreply@anthropic.com>
The top-level TimelineRowsList measures its own unpadded root and shares that width with every action bar through MessageColumnWidthContext, on the premise that each row's [data-message-column] content box equals the list width. The assistant column carries px-2, and useMeasuredWidth reads the content box, so the per-bar observer it replaced reported 16px less for assistant rows. The shared value was therefore 16px too wide for alignment="start" bars — exactly EXPANDED_ROW_COMFORT_PX — and the in-place expansion gate admitted rows it was designed to send to the popover. A start-aligned bar now subtracts PROSE_COLUMN_INSET_PX from the shared width; end-aligned (user) bars keep it, since that column is unpadded. The inset's class and pixel value sit together in MessageActionBar and ConversationMessageContent applies the exported class, so the pair stays in sync. Boundary tests (131px list: popover; 132px: in place) pin the shared path to the threshold the per-bar observer produced, in both the bar unit tests and a ThreadTimelineRows render with the real column markup. Co-authored-by: Vedran Burojevic <vedran.burojevic@gmail.com> Co-Authored-By: Claude <noreply@anthropic.com>
Root cause: notifyHostThreadRuntimeStatusChanged built a full statusChange snapshot one thread at a time (getThread plus buildThreadStatusChangeMetadata, about six SQLite statements each) for every thread joined to the host, including idle, archived and deleted rows whose displayed runtime does not depend on host connectivity. The daemon socket close handler, the disconnect grace callback and host removal all run it synchronously, so a host with a few hundred threads stalled the server event loop for hundreds of milliseconds per disconnect and pushed a snapshot's worth of bytes per row to every thread-list subscriber. Fix: load the host's non-deleted `active` threads with one targeted query (listActiveHostThreads) and build their snapshots in a single batched pass (buildThreadStatusChangeMetadataByThreadId resolves host connectivity and the latest session once and runs the activity helpers over the whole array, as the list endpoints do). Only those rows carry statusChange; every other host thread keeps the bare status-changed push it received before the snapshot existed. The metadata shape is unchanged and shared with buildThreadStatusChangeMetadata. The new test seeds a 4-thread host and a 306-thread host and asserts the daemon-close fan-out issues the same number of SQL statements for both (1234 vs 16 before), that active rows carry the host-reconnecting snapshot, and that idle and deleted rows get a bare push. Co-authored-by: Vedran Burojevic <vedran.burojevic@gmail.com> Co-Authored-By: Claude <noreply@anthropic.com>
The metadata branch of patchThreadListStatusState invalidated the thread-search prefix with cancelRefetch: false but, unlike every other non-cancelling path in the registry, scheduled no trailing refetch. When a status change arrived while a search request was already in flight, TanStack deduped onto that request; its response (read before the transition) then landed and cleared the invalidation, so the search rows kept the old status until some unrelated change refreshed them. Route the prefix through invalidateQueryKeysWithoutCancelingActiveFetches, which keeps the request running and queues one refetch for after it settles. The throttled metadata-less fallback also fed fully-specified list keys through the prefix-matching throttle helpers. Thread list filters are sparse, so a project list key is a prefix of that project's forks-row key: the project key's refetch also fetched the forks list, and the forks key's own run then saw a fetch in flight and scheduled a trailing one, two list requests where main's exact invalidation issued one. The throttle and trailing-refetch helpers now carry an explicit exact flag; leaf list keys enumerated from the cache match exactly, while the sidebar, search and work-status prefixes keep partial matching. Co-authored-by: Vedran Burojevic <vedran.burojevic@gmail.com> Co-Authored-By: Claude <noreply@anthropic.com>
The coarse-pointer case stubs window and resets the module registry to re-import the effects module with a touch pointer class. vitest.shared.ts treats vi.stubGlobal/vi.resetModules as isolation markers, so that one test moved the whole 65-test realtime-cache-effects suite out of the shared node worker into its own isolated worker, paying the module-graph re-import for every run. Keep the case in a small dedicated file so only it runs isolated and the main suite returns to the shared worker. Co-authored-by: Vedran Burojevic <vedran.burojevic@gmail.com> Co-Authored-By: Claude <noreply@anthropic.com>
dirtyThreadSearchQueriesForCompletedTurn had the same gap as the status patch: it invalidated the search prefix with cancelRefetch: false and no trailing refetch, so a turn that completed while a search request was in flight deduped onto that request, whose pre-completion response cleared the invalidation and left the newly indexed content out of the results. Use the same non-cancelling helper so the running request is kept and one refetch follows once it settles. Co-authored-by: Vedran Burojevic <vedran.burojevic@gmail.com> Co-Authored-By: Claude <noreply@anthropic.com>
The shell had moved from `no-cache` to `max-age=300, must-revalidate` so the connect worker could keep a copy in caches.default. But that header is obeyed by every client, not just the worker: a browser, a PWA launch or the desktop window reuses a fresh index.html for up to five minutes without a conditional request (`must-revalidate` only governs stale entries), so after an in-place bb update they booted the previous build whose content-hashed assets now 404 — a blank page until the window ran out. The ETag/304 machinery never ran inside that window, and the "new build picked up immediately" property the comment claimed was gone. The server serves the document `no-cache` again, with the same build-id ETag and If-None-Match -> empty 304 handling, so every navigation on every client is a header-only revalidation and bfcache stays eligible. The connect worker now treats `no-cache` + ETag as the revalidated-shell contract (still rejecting no-store/private/set-cookie; the plain asset path keeps rejecting no-cache). Because caches.default will not hold a `no-cache` response, the stored copy is rewritten to an internal `max-age=300` — dropping content-encoding/content-length, since a subrequest body is read as identity bytes — while the visitor always receives the origin's `no-cache`: on the miss path verbatim, and on the edge-served path by freshening the stored headers with the origin's 304 (RFC 9111 §4.3.4). The stored copy is still only served after the origin confirms its ETag, so the tunnel saving is unchanged. Tests: static-shell.test.ts and static-cache.test.ts pin `no-cache` on the 200 and the 304 (they failed on the previous header); the workerd document-cache test now runs the origin on the `no-cache` contract, asserts every visitor response carries `no-cache` and the stored copy a max-age bound, makes the visitor-304 test self-contained, and pins that a pre-contract server (`no-cache` without a validator) stays uncached. Co-authored-by: Vedran Burojevic <vedran.burojevic@gmail.com> Co-Authored-By: Claude <noreply@anthropic.com>
The "relays the origin's 304 when the visitor presents a current validator" test sent its conditional request for /threads/t1 without ever storing a shell for that path, so it only passed because the preceding test had left build B's copy in caches.default. Run alone (`vitest -t "relays the origin"`) it took the cold path, where serveWithCache forwards the visitor's If-None-Match straight to the origin and returns that 304 without the x-bb-cache marker: `AssertionError: expected null to be 'revalidated'`. The test now navigates cold on its own path (/threads/t2), waits for the edge put, and only then sends the conditional request, so it holds whatever ran before it. The first test pins currentBuild to BUILD_A at its start for the same reason, instead of relying on module-load state. Co-authored-by: Vedran Burojevic <vedran.burojevic@gmail.com> Co-Authored-By: Claude <noreply@anthropic.com>
Merging the boot micro-chunks (plan 007 step 2) cut the measured boot payload to 1,584,595 B raw / 390,065 B brotli (3 chunks), but bundle-budget.json still carried the ceilings computed from the previous 1,566,924 / 435,515 B measurement. That left 23% brotli headroom where the file's own rule says 10%, so ~86 KB brotli of boot regression could have landed with "Bundle budget OK". Set maxBootBrotliBytes to ceil(measured x 1.1): 479,067 -> 429,072. maxBootBytes stays at 1,723,617: the chunk merge grew the raw payload by 17.7 KB while cutting brotli by 45 KB, which leaves 8.8% raw headroom, and a ceiling is never raised without a separate reason. `node apps/app/scripts/check-bundle-budget.mjs` after `turbo run build --filter=@bb/app`: boot payload 1547.5 KB raw / 380.9 KB brotli, budget 1683.2 KB raw / 419.0 KB brotli, OK. Co-authored-by: Vedran Burojevic <vedran.burojevic@gmail.com> Co-Authored-By: Claude <noreply@anthropic.com>
The "emitted dist/index.html head order" suite is describe.skipIf(!existsSync(dist/index.html)). The CI tests job runs `turbo run test --filter=@bb/app` with no build step and the `test` task has no edge to `@bb/app#build`, so that suite reports "1 skipped" on every CI run and the ordering this plugin exists for was only checked locally, after a build. Add a unit test that applies reorderHeadForFirstPaint with resolveFontPreloadTags (the composition the plugin's transformIndexHtml handler performs) to the committed index.html with Vite 8's injected tags: entry script, modulepreloads, stylesheet, one per indented line, the shape dist/index.html shows. It asserts the font preload and the stylesheet follow the pre-paint theme script and precede the entry script and every modulepreload, and that the stylesheet is moved rather than duplicated. The existing fixture has no newlines or indentation, so the regex that consumes Vite's indented stylesheet line was untested. With the insertion anchor swapped back to </head> (the pre-PR order) the new test fails with "expected 4128 to be less than 3777" while the dist-gated suite still passes against the already-built dist; with dist/index.html absent the file runs 8 passed | 1 skipped. The dist-gated suite is kept as-is. Co-authored-by: Vedran Burojevic <vedran.burojevic@gmail.com> Co-Authored-By: Claude <noreply@anthropic.com>
This was referenced Aug 25, 2026
Closed
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
3 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.
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=300app 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 currentmainand 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-bytrailer 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):
Fixes (13 commits; each names the finding it closes):
bottom-anchored-scroll-body.tsx: thedata-scrollbar-scrollingwrite is back on every pointer, idempotent (one write per scroll burst);.thread-scrollbar/.transient-scrollbarare 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 buildstatusChangemetadata only for active threads, fetched in batched queries; idle threads keep the pre-PR bare push.realtime-cache-registry.ts: thestatusChangesearch invalidation keepscancelRefetch: falseand 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 servedCache-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 nowno-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'sno-cache.no-store/private/set-cookie still bypass; the plain asset path still rejectsno-cache. Known limit (unchanged from Boot and delivery quick wins for the relayed mobile path #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:maxBootBrotliBytes479,067 → 429,072 (10% above the measured boot payload);maxBootBytesunchanged.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'spx-2inset; the class and the 16 px constant are declared together.No wire change between server and host daemon (
HOST_DAEMON_PROTOCOL_VERSIONunchanged; #2388's one-linedaemon-protocol.tsedit 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.@bb/db409/409,@bb/connect120/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/server2,047/2,047,@bb/app3,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.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-testssuites 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.