Closes #150#151
Merged
Merged
Conversation
…ithout re-nav (closes #150) Root cause: selectSession() re-runs on every navigation focus (#121's resync), forcing isLoading back to true even when re-selecting the session already shown on screen. That hides the whole conversation (messages + composer) behind a spinner for as long as the redundant GET takes -- while live SSE message/part updates keep flowing to the store the entire time, just invisible behind the spinner. If that GET is slow or stalls, the screen looks permanently "loading"; leaving and re-entering only "fixes" it because it's a fresh retry, not because anything was actually resolved. Fix: only force isLoading=true for a genuinely cold load (no session shown yet, or switching to a different one) via isColdSessionLoad(). A same-session re-focus refreshes in the background without hiding existing (and live-updating) content. As a second safety net, any live message.updated/message.part.updated/session.updated event for the active session now clears isLoading unconditionally via isLiveEventForSession() -- proof-of-life that unsticks the spinner even if the GET itself never resolves. Both are pure, unit-tested in src/lib/session-load-reconcile.ts. Co-Authored-By: Claude Fable 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
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.
Root cause
The session detail screen (
app/session/[id].tsx) re-runsselectSession()on every navigation focus, not just on mount — that's #121's fix, which re-binds the screen to its session on every re-entry because the native stack keeps screens mounted underneath a pushed one.The bug:
selectSession()unconditionally setisLoading: trueat the start of every call, including a re-focus of the session already shown on screen. That hides the entire conversation — message list and composer — behind a spinner for as long as that (redundant)GET /session/:id+GET /session/:id/messagefetch takes.Meanwhile, SSE keeps delivering
message.updated/message.part.updated/session.statusevents the whole time —useEvents'sconnect()loop callsuseSessions.getState().handleEvent(...), which does keep updatingmessages/partsin the store — but the screen can't render any of it whileisLoadingis blocking the message list.If that redundant GET is slow or stalls (flaky mobile network, cold TCP handshake, etc.), the screen looks permanently stuck on "loading" — even though the conversation is progressing live server-side and the store already has (or is receiving) the up-to-date messages. Backing out to the sessions list and re-entering only "fixes" it because it's a fresh retry that happens not to stall the second time — not because anything was actually resolved. This matches the reported repro exactly: stuck loading during an open conversation, fixed only by leaving and re-entering.
Fix
src/stores/sessions.ts:selectSession()now only setsisLoading: truefor a genuinely cold load (no session shown yet, or switching to a different session) — derived via the new pureisColdSessionLoad(currentSessionID, targetSessionID). Re-selecting the session already on screen refreshes in the background without hiding the (already-live-updating) conversation.handleEvent()now clearsisLoadingunconditionally whenever amessage.updated,message.part.updated, orsession.updatedevent lands for the active session — via the new pureisLiveEventForSession(eventSessionID, activeSessionID)(also used to tighten the existingmessage.updatedguard). This unsticks the spinner even in the case where the initial GET never resolves at all.Both are pure functions with no React Native/zustand dependencies, in
src/lib/session-load-reconcile.ts, unit-tested insrc/lib/session-load-reconcile.test.ts(7 cases: cold vs. warm load derivation, and live-event session matching including missing/mismatched ids).Doesn't touch the #121 focus-resync or #123/#124 reconnect-resync code paths —
refreshPending(),resyncBusySessions(), and theuseFocusEffectre-selection itself are all unchanged; only theisLoadinggating insideselectSession/handleEventchanged.Testing
npm test: 216/219 passing (3 pre-existing env-gated cancellations, unrelated to this change), including the 7 newsession-load-reconcile.test.tscases.npx tsc --noEmit: clean.No device test was run (none available in this environment) — reasoned through the state flow instead: reviewed every call site of
selectSession/handleEvent, confirmed the only trigger forselectSessionisuseFocusEffectinapp/session/[id].tsx, and confirmed the fix can't leaveisLoadingstuck (it's only ever forcedtruefor a cold load, and always eventually gets setfalseon that load's own success/failure path, plus the new live-event safety net).On-device verify steps
🤖 Generated with Claude Code