Realtime round 2: status-change metadata everywhere, reconnect gating, coalesced fallback refetches - #2388
Closed
vburojevic wants to merge 5 commits into
Closed
Conversation
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 get-bb#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>
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>
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>
createAppQueryClient gated the focus refetch (get-bb#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>
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>
This was referenced Aug 25, 2026
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.
|
This was referenced 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>
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
Four realtime fan-outs survived the #1880–#1901 and #2277–#2281 rounds. A
status-changedpush withoutstatusChangemetadata makes every client refetch every active thread list + sidebar bootstrap (~1 KB per unarchived thread) on the un-debounced immediate path — and the host-connectivity fan-out published exactly such bare pushes once per thread on the host. The same immediate path cancelled in-flight search requests, which can starve an open search on a slow link.refetchOnReconnectwas left at TanStack's defaulttrue, so every mobile-Safarionlineblip re-fired the wave #2278 gated for focus. And each in-flight timeline row owned its own 1 HzsetIntervalwhile streaming reconciled the unwindowed list up to 20×/s on phones.What changed
notifyHostThreadRuntimeStatusChanged(daemon close, disconnect grace, host removal) and the post-commitinterruptActiveThreadspublish attach the row snapshot via the existingbuildThreadStatusChangeMetadatabuilder; in-transaction sites (bufferedDbNotifier, no runtime hub) are annotated as deliberately bare.thread-send/parent-system-messages/queued auto-send already attached it since Patch sidebar rows from status-changed pushes instead of refetching #2169. No daemon wire change.cancelRefetch: false.createAppQueryClientapplies the lost-realtime-coverage gate torefetchOnReconnect(same predicate as the focus gate); the two policies with explicitrefetchOnReconnect: truekeep it with re-justification comments.LiveDurationTextrides the shareduseSecondTickticker, which now stops while the document is hidden and ticks immediately on resume. The realtime invalidation debounce readsmatchMedia("(pointer: coarse)")once at module init: touch devices widen 50/200ms to 150/400ms; desktop cadence unchanged (pinned by test).How you verified
New tests proven fail-before/pass-after: daemon-close and host-removal pushes carry
statusChange; two bare pushes within 1s produce one active refetch; a status patch no longer aborts an in-flight search holding cached data; anonlineevent doesn't refetch while realtime coverage is intact and does once lost; the shared ticker pauses hidden.pnpm exec turbo run typecheck --filter=@bb/app --filter=@bb/serverexit 0; server suites 63 files / 502 tests; app hooks suites 59 files / 376; timeline+banner suites 37 files / 292. Green on the merged integration tree (server 2,044).