Boot and delivery quick wins for the relayed mobile path - #2389
Closed
vburojevic wants to merge 5 commits into
Closed
Boot and delivery quick wins for the relayed mobile path#2389vburojevic wants to merge 5 commits into
vburojevic wants to merge 5 commits into
Conversation
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>
…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>
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>
…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>
…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>
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.
What was wrong
A thread page on the relayed mobile path (CF worker → Durable Object → tunnel WebSocket → laptop) was 113 requests / ~3.5 MB raw JS, and the tunnel serializes responses FIFO — so discovery order and request count are the latency. Four root causes: the render-blocking stylesheet was the 68th resource discovered in the built
index.html(font preload dead last); no chunk-merging config existed (70 boot chunks, half carrying ~2% of the bytes, 18 below the precompress floor);index.htmlwasno-cache— which the connect worker treats as never-cacheable — with no brotli sidecar, and the SPA fallback bypassed the precompressed path entirely; the connect gate's per-isolate caches stored only settled values, so a cold isolate's burst paid one D1 round trip per request instead of per key.What changed
vite-font-preload.ts: the stylesheet (withfetchpriority="high") and font preload now precede the entry script and modulepreload block; the transform fails the build if the stylesheet would ever precede the pre-paint theme script.vite.config.ts:advancedChunkswith two$initial-tagged groups (vendor/app) merges the boot graph 70 → 3 chunks (boot brotli 443.0 → 381.2 KB); lazy-route and on-demand facades untouched, all budget gates hold.precompress-app-dist.mjs:.htmladded (index.html.brships); the server's SPA fallback unified with the sidecar-aware serving path.max-age=300, must-revalidateplus a weak build-id ETag (sha256 of the served file — index.html embeds every hashed asset URL, so its content hash IS the build id) and answers If-None-Match with 304. The connect worker keeps the last confirmed document incaches.defaultand revalidates through the tunnel on every navigation, so new builds still land on the next navigation; storage bounded ≤300s.session.ts: label/session caches store the in-flight promise (single-flight); rejected lookups evicted on settle; TTLs unchanged. Rebased over Refresh active bb Connect sessions #2357 — its session-refresh semantics preserved (identity-guarded settle handlers; edge-served shells deliberately don't count as session activity).Deploy note: the connect worker deploys separately; either order is safe (bounded ≤300s skew), worker-first avoids even that.
How you verified
Fail-before/pass-after: emitted head-order test against the real
dist/index.html(expected 9654 to be less than 3861before); single-flight burst tests (expected 6 to be 1D1 selects) for both lookups. Document cache proven in real workerd via the tunnel fixture (repeat navigation = edge body + 304 on the tunnel; build change ships next navigation; visitor 304 relayed). Bundle budget OK before/after (boot 70→3 chunks). After the #2357 rebase:@bb/connect119/119 including main's new session-refresh suite; head-order/precompress/plugin-frontend suites 54/54;check:bundleOK. Green on the merged integration tree.