Skip to content

Commit c21820e

Browse files
SawyerHoodvburojevicclaudekirbyhood
authored
Mobile perf: integrate #2385, #2386, #2388, #2389, #2392 with the review 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>
1 parent 71c56b0 commit c21820e

62 files changed

Lines changed: 5065 additions & 468 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎apps/app/bundle-budget.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
"to print the static chain that pulled a package into the closure."
4848
],
4949
"maxBootBytes": 1723617,
50-
"maxBootBrotliBytes": 479067,
50+
"maxBootBrotliBytes": 429072,
5151
"forbiddenBootPackages": [
5252
"@pierre/diffs",
5353
"@pierre/theming",

‎apps/app/src/components/layout/useMobileVisualViewportHeight.test.tsx‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,18 @@ function withElementClientHeight(
108108
}
109109
}
110110

111+
// Waits out one scheduled rAF pass so "the pass ran and did nothing" is
112+
// distinguishable from "the pass has not run yet".
113+
async function flushScheduledViewportPass() {
114+
await act(async () => {
115+
await new Promise<void>((resolve) => {
116+
window.requestAnimationFrame(() => {
117+
window.requestAnimationFrame(() => resolve());
118+
});
119+
});
120+
});
121+
}
122+
111123
beforeEach(() => {
112124
vi.spyOn(window, "scrollTo").mockImplementation(() => {});
113125
});
@@ -310,6 +322,142 @@ describe("useMobileVisualViewportHeight", () => {
310322
expect(window.scrollTo).not.toHaveBeenCalled();
311323
});
312324
});
325+
326+
it("writes shell geometry only when a pass computes new values", async () => {
327+
const visualViewport = new FakeVisualViewport();
328+
visualViewport.offsetTop = 0;
329+
await withFakeVisualViewport(visualViewport, async () => {
330+
render(<VisualViewportShell enabled />);
331+
const shell = screen.getByTestId("shell");
332+
const shellHeightRoot = screen.getByTestId("shell-height-root");
333+
expect(shell.style.height).toBe("500px");
334+
const setShellHeightProperty = vi.spyOn(
335+
shellHeightRoot.style,
336+
"setProperty",
337+
);
338+
339+
// Same geometry again: the pass must return before any style write, or
340+
// every keyboard/URL-bar animation frame invalidates the whole tree.
341+
act(() => {
342+
visualViewport.dispatchEvent(new Event("resize"));
343+
});
344+
await flushScheduledViewportPass();
345+
expect(setShellHeightProperty).not.toHaveBeenCalled();
346+
347+
act(() => {
348+
visualViewport.height = 480;
349+
visualViewport.dispatchEvent(new Event("resize"));
350+
});
351+
await waitFor(() => expect(shell.style.height).toBe("480px"));
352+
expect(setShellHeightProperty).toHaveBeenCalledTimes(1);
353+
});
354+
});
355+
356+
it("reads the containing block only when the layout viewport can change", async () => {
357+
const visualViewport = new FakeVisualViewport();
358+
visualViewport.offsetTop = 0;
359+
let containingBlockReads = 0;
360+
await withElementClientHeight(
361+
document.body,
362+
() => {
363+
containingBlockReads += 1;
364+
return 800;
365+
},
366+
async () =>
367+
withFakeVisualViewport(visualViewport, async () => {
368+
render(<VisualViewportShell enabled />);
369+
const shell = screen.getByTestId("shell");
370+
expect(shell.style.height).toBe("500px");
371+
const readsAfterMount = containingBlockReads;
372+
373+
// Visual-viewport ticks pan or resize only the visual viewport;
374+
// they must reuse the cached containing-block height instead of
375+
// forcing a full-document layout per animation frame.
376+
act(() => {
377+
visualViewport.offsetTop = 40;
378+
visualViewport.dispatchEvent(new Event("scroll"));
379+
});
380+
await waitFor(() => expect(shell.style.top).toBe("40px"));
381+
act(() => {
382+
visualViewport.height = 460;
383+
visualViewport.dispatchEvent(new Event("resize"));
384+
});
385+
await waitFor(() => expect(shell.style.height).toBe("460px"));
386+
expect(containingBlockReads).toBe(readsAfterMount);
387+
388+
act(() => {
389+
window.dispatchEvent(new Event("resize"));
390+
});
391+
await waitFor(() =>
392+
expect(containingBlockReads).toBe(readsAfterMount + 1),
393+
);
394+
}),
395+
);
396+
});
397+
398+
it("runs a geometry pass when an editor is focused programmatically", async () => {
399+
const visualViewport = new FakeVisualViewport();
400+
visualViewport.offsetTop = 0;
401+
await withElementClientHeight(
402+
document.body,
403+
() => 500,
404+
async () =>
405+
withFakeVisualViewport(visualViewport, async () => {
406+
render(<VisualViewportShell enabled />);
407+
const shell = screen.getByTestId("shell");
408+
const editor = screen.getByTestId("editor");
409+
// Native layout matches the visual viewport: no override applied.
410+
expect(shell.style.height).toBe("");
411+
412+
// The keyboard shortens the visual viewport around the same time
413+
// the composer autofocuses, without any window resize; the focus
414+
// pass must pick the change up on its own.
415+
visualViewport.height = 300;
416+
act(() => editor.focus());
417+
await waitFor(() => expect(shell.style.height).toBe("300px"));
418+
}),
419+
);
420+
});
421+
422+
it("ignores visual viewport pans without a keyboard or an applied override", async () => {
423+
const visualViewport = new FakeVisualViewport();
424+
visualViewport.offsetTop = 0;
425+
await withElementClientHeight(
426+
document.body,
427+
() => 500,
428+
async () =>
429+
withFakeVisualViewport(visualViewport, async () => {
430+
render(<VisualViewportShell enabled />);
431+
const shell = screen.getByTestId("shell");
432+
const editor = screen.getByTestId("editor");
433+
expect(shell.style.height).toBe("");
434+
435+
// A URL-bar pan with no keyboard: nothing to compensate.
436+
act(() => {
437+
visualViewport.offsetTop = 340;
438+
visualViewport.dispatchEvent(new Event("scroll"));
439+
});
440+
await flushScheduledViewportPass();
441+
expect(window.scrollTo).not.toHaveBeenCalled();
442+
expect(shell.style.top).toBe("");
443+
444+
// With a keyboard editor focused, the same pan is Safari's
445+
// focus-reveal pan and must still be compensated. Let the pan
446+
// settle and the focus-scheduled pass run first, so that only the
447+
// scroll handler's keyboard branch can produce the compensation.
448+
visualViewport.offsetTop = 0;
449+
act(() => editor.focus());
450+
await flushScheduledViewportPass();
451+
expect(shell.style.top).toBe("");
452+
act(() => {
453+
visualViewport.offsetTop = 340;
454+
visualViewport.dispatchEvent(new Event("scroll"));
455+
});
456+
await waitFor(() => expect(shell.style.top).toBe("340px"));
457+
expect(window.scrollTo).toHaveBeenCalledWith(0, 0);
458+
}),
459+
);
460+
});
313461
});
314462

315463
describe("shouldRestoreIOSViewportOnKeyboardDismissal", () => {

‎apps/app/src/components/layout/useMobileVisualViewportHeight.ts‎

Lines changed: 68 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,22 @@ export function useMobileVisualViewportHeight(
5555
if (!shell || !shellHeightRoot || !enabled || !visualViewport) return;
5656

5757
let animationFrame: number | null = null;
58+
// The override last written to the shell, or null while none is applied.
59+
// Writing shell `top`/`height` and the inherited `--bb-shell-height`
60+
// invalidates computed style for the whole app tree, and passes run at
61+
// visual-viewport event cadence (keyboard animation, URL-bar collapse),
62+
// so a pass that recomputes unchanged geometry must not write at all.
63+
let appliedOverride: { top: number; height: number } | null = null;
64+
// Reading `document.body.clientHeight` forces a full-document layout. The
65+
// shell's containing block only changes when the layout viewport does, so
66+
// cache the read and mark it stale only on triggers that can resize the
67+
// layout viewport — never on visualViewport ticks, which move or resize
68+
// only the visual viewport.
69+
let shellContainingBlockHeight = 0;
70+
let shellContainingBlockHeightStale = true;
5871
const clearViewportOverride = () => {
72+
if (appliedOverride === null) return;
73+
appliedOverride = null;
5974
shell.style.removeProperty("top");
6075
shell.style.removeProperty("height");
6176
shellHeightRoot.style.removeProperty("--bb-shell-height");
@@ -68,12 +83,15 @@ export function useMobileVisualViewportHeight(
6883
}
6984

7085
const visualViewportHeight = Math.round(visualViewport.height);
71-
// `documentElement.clientHeight` is the visible viewport height for the
72-
// root element, even when that root's actual CSS box extends behind an
73-
// Android in-app browser toolbar. The body inherits the root box and
74-
// therefore exposes the containing-block height the app shell really
75-
// receives.
76-
const shellContainingBlockHeight = document.body.clientHeight;
86+
if (shellContainingBlockHeightStale) {
87+
// `documentElement.clientHeight` is the visible viewport height for the
88+
// root element, even when that root's actual CSS box extends behind an
89+
// Android in-app browser toolbar. The body inherits the root box and
90+
// therefore exposes the containing-block height the app shell really
91+
// receives.
92+
shellContainingBlockHeight = document.body.clientHeight;
93+
shellContainingBlockHeightStale = false;
94+
}
7795
const hasVisualViewportPan =
7896
visualViewport.offsetTop > 1 || window.scrollY > 0;
7997
if (
@@ -91,7 +109,16 @@ export function useMobileVisualViewportHeight(
91109
// compensation below also handles a visual-viewport-only pan.
92110
window.scrollTo(0, 0);
93111
}
94-
shell.style.top = `${getVisualViewportPageTop(visualViewport)}px`;
112+
const shellTop = getVisualViewportPageTop(visualViewport);
113+
if (
114+
appliedOverride !== null &&
115+
appliedOverride.top === shellTop &&
116+
appliedOverride.height === visualViewportHeight
117+
) {
118+
return;
119+
}
120+
appliedOverride = { top: shellTop, height: visualViewportHeight };
121+
shell.style.top = `${shellTop}px`;
95122
shell.style.height = `${visualViewportHeight}px`;
96123
// Fixed-position descendants cannot inherit the shell element's pixel
97124
// height. Publish the same correction through the existing shell-height
@@ -108,6 +135,28 @@ export function useMobileVisualViewportHeight(
108135
}
109136
animationFrame = window.requestAnimationFrame(updateHeight);
110137
};
138+
// For triggers where the layout viewport may have changed: window resize,
139+
// rotation, and an editor gaining focus — the pass that sizes the shell
140+
// for the arriving keyboard must start from the real containing block,
141+
// and these triggers are rare enough that the forced layout is fine.
142+
const scheduleContainingBlockUpdate = () => {
143+
shellContainingBlockHeightStale = true;
144+
scheduleUpdate();
145+
};
146+
const handleVisualViewportScroll = () => {
147+
// Keyboard-less visual-viewport pans (URL-bar collapse, momentum
148+
// settling) don't change the containing block and need no override —
149+
// the pan compensation exists for the keyboard focus-reveal pan. Only
150+
// an already-applied override still has to track pans, because embedded
151+
// browsers apply one without any keyboard.
152+
if (
153+
appliedOverride === null &&
154+
!isKeyboardFocusTarget(document.activeElement)
155+
) {
156+
return;
157+
}
158+
scheduleUpdate();
159+
};
111160

112161
// Safari with its bottom toolbar visible does not update the visual
113162
// viewport until the keyboard animation ends. Restore the normal shell
@@ -124,22 +173,29 @@ export function useMobileVisualViewportHeight(
124173
};
125174
const handleFocusIn = (event: FocusEvent) => {
126175
if (!isKeyboardFocusTarget(event.target)) return;
127-
scheduleUpdate();
176+
// Programmatic focus (composer autofocus) can be the only trigger for a
177+
// keyboard, so this must always schedule a full, freshly measured pass.
178+
scheduleContainingBlockUpdate();
128179
};
129180

130181
updateHeight();
131182
visualViewport.addEventListener("resize", scheduleUpdate);
132-
visualViewport.addEventListener("scroll", scheduleUpdate);
133-
window.addEventListener("resize", scheduleUpdate);
183+
visualViewport.addEventListener("scroll", handleVisualViewportScroll);
184+
window.addEventListener("resize", scheduleContainingBlockUpdate);
185+
window.addEventListener("orientationchange", scheduleContainingBlockUpdate);
134186
if (restoreImmediatelyOnKeyboardDismissal) {
135187
document.addEventListener("focusout", handleFocusOut);
136188
document.addEventListener("focusin", handleFocusIn);
137189
}
138190

139191
return () => {
140192
visualViewport.removeEventListener("resize", scheduleUpdate);
141-
visualViewport.removeEventListener("scroll", scheduleUpdate);
142-
window.removeEventListener("resize", scheduleUpdate);
193+
visualViewport.removeEventListener("scroll", handleVisualViewportScroll);
194+
window.removeEventListener("resize", scheduleContainingBlockUpdate);
195+
window.removeEventListener(
196+
"orientationchange",
197+
scheduleContainingBlockUpdate,
198+
);
143199
if (restoreImmediatelyOnKeyboardDismissal) {
144200
document.removeEventListener("focusout", handleFocusOut);
145201
document.removeEventListener("focusin", handleFocusIn);

‎apps/app/src/components/thread/timeline/ConversationMessageContent.tsx‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,10 @@ import {
5454
import { turnRequestLabel } from "@bb/client-core";
5555
import { splitStreamingMarkdown } from "./streaming-markdown-split.js";
5656
import { TurnRequestLabel } from "./TurnRequestLabel.js";
57-
import { MessageActionBar } from "./MessageActionBar.js";
57+
import {
58+
MessageActionBar,
59+
PROSE_COLUMN_INSET_CLASS,
60+
} from "./MessageActionBar.js";
5861
import {
5962
ConversationMessageOverflowToggle,
6063
useIsOverflowing,
@@ -678,7 +681,10 @@ function AssistantConversationMessage({
678681

679682
return (
680683
<div
681-
className="group/message w-full px-2 text-sm font-normal leading-relaxed"
684+
className={cn(
685+
"group/message w-full text-sm font-normal leading-relaxed",
686+
PROSE_COLUMN_INSET_CLASS,
687+
)}
682688
data-message-column=""
683689
>
684690
{/*

0 commit comments

Comments
 (0)