perf(timeline): the reader-id fix shipped in a function the page never calls - #826
Merged
Merged
Conversation
getCurrentUserId was defined six times across services - timeline queries, timeline processors, groups, loans, projects, and the auth layer. Every copy called supabase.auth.getUser(), which is a NETWORK call: it validates the token against /auth/v1/user. So a single page asked the server who the reader was over and over, and the cache I added yesterday covered only four of the six importers. On the timeline that lookup also sat on the critical path. Measured on a cold load: 2429 -> 3061 get_user_timeline_feed 632ms 3105 -> 3351 auth/v1/user 246ms <- depends on nothing 3367 -> 3595 reactions x3 228ms <- waits on the one above The identity lookup ran after the feed returned, and then the reaction queries waited on it in turn, though it depends on neither. Changes: - One definition, in the auth layer, caching the in-flight promise so concurrent callers collapse onto one request. Safe as module state only because this module talks exclusively to the browser client; a per-request server client would hand one request's user to the next, and the one caller that passes its own client (groups) keeps its uncached path. - The cache is dropped on any auth state change, registered lazily on first use rather than at import - this module is imported very widely and an import-time subscription is a side effect every importer pays for. - getUserFeed warms the id alongside the feed request instead of after it, and asks for the total count concurrently rather than after enrichment. - check:one-current-user fails the build if a seventh copy appears. Wired into verify. A test caught a real behaviour change while writing this: getUser() catches its own errors and reports them in `error` rather than throwing, so "could not ask" arrived looking like "nobody is signed in" and would have been cached as signed-out for the rest of the page. Fixed the code, not the test. Mutation-proved: a seventh definition -> red; the owner losing its definition -> red; both green after restore; and the gate is reached through npm, not just when run by hand. Full suite: 261 suites, 2524 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012dpTLxh5GJWeWTF1UEvcD5
…hrough My previous commit warmed the reader's id inside getUserFeed. The timeline page calls getEnrichedUserFeed. So the fix shipped to production and did nothing: /auth/v1/user still started at 3552ms, after the feed returned at 3520, with the three reaction queries queued behind it. Moved to the timeline service facade, which every read goes through - including getEventById, getReplies, searchPosts and getThreadPosts, which have the same ordering problem on thread pages. One rule in one place rather than a call repeated inside each query function, and no longer dependent on me picking the right one. The test asserts the ORDER, not that the warm happens: a test that only checked "it was called" would pass with the call left at the end, which is the bug. It runs over every read the facade exposes and fails if a new one is added without warming. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012dpTLxh5GJWeWTF1UEvcD5
github-actions Bot
pushed a commit
that referenced
this pull request
Aug 28, 2026
…ed page (#827) warmCurrentUserId is called at the top of every timeline read, and timeline reads happen during server rendering too. It started an unconditional /auth/v1/user request from the SERVER using the BROWSER Supabase client, which carries no request cookies - so it could only ever fail, and it failed while the route was rendering. That is not a wasted request, it is an outage. /timeline, /community and /dashboard sat on their route-level loading.tsx skeleton forever on a fresh page load. Navigating to the very same route from inside the app worked, because that path never server-renders - which is what made it look intermittent. The skeleton is near-white on a white page, so the symptom users see is "the timeline is blank". Nothing in the console, no failed request, every check green. I introduced this in #826 and did not catch it, because I verified the fix on the post page and by measuring the network waterfall, neither of which exercises a cold server render of a feed route. The warm is now a no-op on the server, silently: it is an optimisation, and an optimisation must never be the reason a page fails. The test runs in the `node` jest environment on purpose. Under jsdom `window` exists, the guard is never exercised, and the test would pass while the server path stayed broken - which is exactly how this shipped. Mutation-proved: removing the guard turns it red. Claude-Session: https://claude.ai/code/session_012dpTLxh5GJWeWTF1UEvcD5 Co-authored-by: Georgy Butaev <41178744+g-but@users.noreply.github.com> Co-authored-by: Claude Opus 5 <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.
Follow-up to #824. Two things that PR did not actually deliver.
What happened
#824 was squash-merged at the head the auto-merge sweep saw, which was one push behind — the
getCurrentUserIdconsolidation never reached main. Verified againstorigin/main: the level-ordergetReplieslanded, the consolidation did not.And the part that was written had a bug of its own: I warmed the reader's id inside
getUserFeed. The timeline page callsgetEnrichedUserFeed. So even had it merged, it would have done nothing. Confirmed live after that deploy:This PR
1. The consolidation (unchanged from #824, re-applied).
getCurrentUserIdwas defined six times — timeline queries, timeline processors, groups, loans, projects, auth — each an uncached call to/auth/v1/user. Now one definition in the auth layer, caching the in-flight promise so concurrent callers collapse onto one request. Safe as module state only because that module talks exclusively to the browser client; the one caller that passes its own client (groups) keeps its uncached path. Cache is dropped on any auth state change, registered lazily on first use rather than at import.check:one-current-userfails the build on a seventh copy.2. The warm, moved to a door that all reads pass through. The timeline service facade — so it covers
getEnrichedUserFeed,getEnrichedFollowingFeed,getCommunityFeed, and alsogetEventById/getReplies/searchPosts/getThreadPosts, which have the same ordering problem on thread pages. One rule in one place, and no longer dependent on me guessing which function the page calls.Tests assert the ORDER
A test that only checked "the warm happened" would pass with the call left at the end — which is exactly the bug. So the test records call order across every read the facade exposes, and fails if a new read is added without warming.
Mutation-proved — each red, then green after restore:
getEnrichedUserFeedstops warminggetCurrentUserIddefinitionThe gate was also confirmed reachable through
npm run, not only by hand.Already verified live from #824
The level-order traversal works: opening a 5-reply thread now makes 1
/auth/v1/usercall (was 8) and exactly 3timeline_eventsqueries — one per depth, not one per reply.