perf: patch mobile list rows from status-changed pushes instead of refetching (M1) - #2350
Open
SawyerHood wants to merge 3 commits into
Open
perf: patch mobile list rows from status-changed pushes instead of refetching (M1)#2350SawyerHood wants to merge 3 commits into
SawyerHood wants to merge 3 commits into
Conversation
…f refetching Every status-changed push invalidated the sidebar bootstrap (staleTime Infinity) and the thread lists, so each agent turn re-downloaded the whole sidebar at least twice per thread although the push already carried the rewritten row (metadata.statusChange, server #2169) and mobile parsed it. Mirror the web's identity-guarded patch path: write the five row fields into every cached list and the sidebar, invalidate in-flight list fetches synchronously with exact: true so a stale response cannot overwrite the patch, and keep the list refetch for bare pushes (stop/interrupt/host loss). Verified: 7 new vitest cases fail on HEAD and pass after; turbo typecheck for @bb/mobile passes; oxlint/oxfmt clean. Co-Authored-By: Claude <noreply@anthropic.com>
…ht list restarts Review of c133e912d found three behavioural gaps in the status-changed patch path and a weak test. (1) applyToCachedThreadLists / applyToCachedSidebarThreads called setQueryData on every cached list and the sidebar even when the mapper returned the list untouched. query-core's success dispatch clears isInvalidated and resets dataUpdatedAt, so every row-carrying status-changed push for any thread silently dropped the refetch-on-remount flag of inactive lists (archived list, forks / typeahead lists) that an earlier archived-/title-/pin-changed push had set, and an errored sidebar stopped retrying on focus. Now a list the mapper left referentially unchanged is not written at all (mapThreadListCacheData / mapSidebarBootstrapThreads return their input on identity), and a real write preserves dataUpdatedAt (setQueryData with updatedAt) and re-arms query.invalidate() when the query was invalidated before. This applies to every caller of the helper (optimistic patches included): a client-side patch is not fresher server data. (2) applyThreadStatusChangeFromMessage restarted every list fetch in flight once per push (invalidateQueries with the default cancelRefetch), so a burst of row-carrying pushes (a workflow fanning out children) aborted and restarted the ~133 KB sidebar bootstrap once per push. It now aborts the fetch synchronously (cancelQueries with revert:false, silent:true, so the patch is kept and no error state is entered), flags it stale with refetchType 'none', and returns the keys; installRealtimeInvalidation queues them in the debounced pending map as exact entries (PendingInvalidation.exact) so the restart happens once per flush. A prefix invalidation of the same lists in the same window subsumes the exact restart (partialMatchKey) so the flush does not start and abort-and-restart the fetch in one tick. Because cancelQueries, unlike cancelRefetch, has no data gate, getFetchingThreadListQueryKeys now skips first loads (no data yet): they hold no patched row and were never restarted before either. (3) The in-flight test ran under setup()'s mocked invalidateQueries and asserted only the first argument, so the cancel/restart guarantee was not exercised. It now restores the spy, drives a real sidebar refetch through a QueryObserver, and asserts one abort, the restart on the flush, and that the late stale body does not overwrite the patched row. (4) The updateCachedThreadListStatusState doc no longer claims the sidebar rows key on array identity (mobile rebuilds rows from the bootstrap; the guard skips the copy and the write). Verified: with the HEAD sources swapped in, the 8 new/strengthened cases fail (4 in thread-list-cache.test.ts, 3 in realtime-invalidation.test.ts, plus the first-load case of getFetchingThreadListQueryKeys) and 23 pass; with the fix, the five related mobile files pass (49 tests). turbo typecheck for @bb/mobile passes; oxlint/oxfmt clean. Co-Authored-By: Claude <noreply@anthropic.com>
…rts in the flush
Round-2 review of 3e9ac4c05 found that applyThreadStatusChangeFromMessage aborted an in-flight list/sidebar fetch with cancelQueries({ revert: false, silent: true }). In query-core 5.90.20 that rejects the retryer but dispatches nothing, so the query stayed at fetchStatus 'fetching' with no request behind it until the flush's invalidateQueries({ exact: true }) restarted it - and that restart is active-only. A query whose observer unsubscribed (ArchivedThreadsScreen popped) or flipped to enabled: false (the typeahead's useThreadsList) inside the 50-200 ms debounce window was stranded: never garbage-collected (optionalRemove needs 'idle'), re-selected by getFetchingThreadListQueryKeys on every later push (no-op cancel/invalidate plus a queued restart that never ran), and reported as fetching to the next observer until a remount. The doc comment's claim that a revert would drop the patch was also wrong: writeCachedListPatch is a manual setQueryData, and query-core moves the revert point to the last manual write.
The cancel now uses the default revert ({ silent: true }): it lands on the patched rows at fetchStatus 'idle', the following refetchType 'none' invalidate re-flags the query stale, and the query is collectable and restartable by any later observer or fetchQuery. In the flush, an exact restart entry whose query has no enabled observer is skipped explicitly instead of being handed to invalidateQueries' active-only refetch: the query is already idle and stale from the abort, so the next mount, enable, or fetchQuery refetches it. The verifier's additional setState({ fetchStatus: 'idle' }) guard is not applied: with the reverting cancel, a 'fetching' inactive query always has a live request (e.g. an observer-less fetchQuery started after the patch), and forcing it idle would misreport that request and make the next mount start a duplicate.
Tests: the in-flight helper is generalized over the query key (refetchInFlight, with an enabled toggle); the existing in-flight case now asserts fetchStatus 'idle' after the abort; two new cases cover an observer that leaves before the flush (idle, not re-selected, collected after gcTime) and one disabled before the flush (idle and stale, no restart invalidate issued, not re-selected by a later push, refetched on re-enable); a third puts a filtered ['threads', ...] fetch in flight and asserts the ['threads'] prefix invalidation subsumes the exact restart (one restart, one abort). The test client sets gcTime to the on-device default because query-core defaults it to Infinity under node. Verified: against HEAD's source the strengthened case and both stranded cases fail (25 others pass); against a variant with only the cancel change the disabled-observer case fails on the restart-invalidate assertion; swapping partialMatchKey's arguments fails only the prefix case (3 queryFn calls instead of 2); with the fix both M1 test files pass (34 tests), turbo typecheck for @bb/mobile passes, oxlint/oxfmt clean.
Co-Authored-By: Claude <noreply@anthropic.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
1 participant
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
The Expo app refetched the whole sidebar on every status change (an internal performance sweep, finding M1):
status-changedis inTHREAD_LIST_AFFECTING_KINDS, so each push invalidated the sidebar query (staleTime: Infinity, 14 observers) and triggered a fullGET /sidebar-bootstrap— 133 KB at least twice per agent turn per thread. The server already attachesmetadata.statusChange(#2169) and the app parsed it, then never read it.What changed
apps/mobile/src/data/threads/thread-list-cache.ts,apps/mobile/src/lib/query/realtime-invalidation.ts(mirrors the web's #2169 path):updateCachedThreadListStatusStatepatches the matching row in every cached thread list and the sidebar with the web's identity guard (if (!list.some(t => t.id === threadId)) return list). Lists the mapper leaves untouched are not written at all; real writes keep the query'sdataUpdatedAtand re-arm a pending invalidation, so inactive lists do not lose their refetch-on-remount flag.status-changedpush that carries the row no longer invalidates the list keys (only the thread and search keys); bare pushes (stop/interrupt, provisioning, cleanup) still refetch the lists.idle) and restarted once per debounced flush; a burst of pushes restarts the 133 KB bootstrap once. Exact restarts are skipped when a prefix invalidation in the same flush covers them, and when the query has no active observer (it stays stale and idle for the next mount).HOST_DAEMON_PROTOCOL_VERSIONbump.How you verified
New tests in
apps/mobile/src/lib/query/realtime-invalidation.test.tsandapps/mobile/src/data/threads/thread-list-cache.test.ts, each failing on the pre-fix code: row patch through the real socket bridge (sidebar and flat list rows updated, other project'sthreadsarray identity kept); key mapping (row-carrying push → thread + search keys only; bare push and mixed kinds → list keys); no-op patch leaves query state,dataUpdatedAtandisInvalidateduntouched; errored invalidated sidebar stays invalidated; in-flight sidebar refetch aborted once and restarted on the flush with the stale body dropped; eight-push burst → one abort, one restart; observer leaving before the flush leaves the query idle and collectable; disabled observer refetches on re-enable; prefix invalidation subsumes an exact restart. Verified against@tanstack/query-core5.90.20 semantics (revert,silent,refetchType: "none",isActive).On this branch:
pnpm exec turbo run typecheck(76/76),pnpm exec turbo run lint,pnpm exec turbo run test(all packages green). Not done: a device run (tests are node/jsdom, not Hermes). Suggested manual check before merge: watch the sidebar through an agent turn, open the archived list and the @-typeahead, background/foreground the app.Fixes: no issue — source is an internal performance sweep (finding M1).