fix(sessions): live-update open session screen; clear stuck loading without re-nav (closes #150) (#151)
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: test <test@test.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
34
src/lib/session-load-reconcile.test.ts
Normal file
34
src/lib/session-load-reconcile.test.ts
Normal file
@@ -0,0 +1,34 @@
|
|||||||
|
import { test } from "node:test"
|
||||||
|
import assert from "node:assert/strict"
|
||||||
|
import { isColdSessionLoad, isLiveEventForSession } from "./session-load-reconcile.ts"
|
||||||
|
|
||||||
|
test("isColdSessionLoad: no session shown yet -> cold (needs the spinner)", () => {
|
||||||
|
assert.equal(isColdSessionLoad(undefined, "s1"), true)
|
||||||
|
assert.equal(isColdSessionLoad(null, "s1"), true)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("isColdSessionLoad: switching to a different session -> cold", () => {
|
||||||
|
assert.equal(isColdSessionLoad("s1", "s2"), true)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("isColdSessionLoad: re-selecting the session already on screen -> not cold (background refresh)", () => {
|
||||||
|
assert.equal(isColdSessionLoad("s1", "s1"), false)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("isLiveEventForSession: matching ids -> true", () => {
|
||||||
|
assert.equal(isLiveEventForSession("s1", "s1"), true)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("isLiveEventForSession: mismatched ids -> false", () => {
|
||||||
|
assert.equal(isLiveEventForSession("s1", "s2"), false)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("isLiveEventForSession: missing event session id -> false", () => {
|
||||||
|
assert.equal(isLiveEventForSession(undefined, "s1"), false)
|
||||||
|
assert.equal(isLiveEventForSession(null, "s1"), false)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("isLiveEventForSession: missing active session id -> false", () => {
|
||||||
|
assert.equal(isLiveEventForSession("s1", undefined), false)
|
||||||
|
assert.equal(isLiveEventForSession("s1", null), false)
|
||||||
|
})
|
||||||
44
src/lib/session-load-reconcile.ts
Normal file
44
src/lib/session-load-reconcile.ts
Normal file
@@ -0,0 +1,44 @@
|
|||||||
|
/**
|
||||||
|
* Decide whether selecting `targetSessionID` for the session detail screen
|
||||||
|
* is a "cold" load — nothing is shown yet, or we're switching to a
|
||||||
|
* different session, so the blocking spinner is appropriate — or a
|
||||||
|
* background refresh of a session that's already loaded and rendering.
|
||||||
|
*
|
||||||
|
* Why this exists (issue #150): the session detail screen re-runs
|
||||||
|
* `selectSession` on every navigation focus (see #121's useFocusEffect
|
||||||
|
* resync, which re-binds this screen to its session on every re-entry, not
|
||||||
|
* just mount — since the native stack keeps screens mounted underneath a
|
||||||
|
* pushed one). That re-fetch is a good idea (it recovers from missed SSE
|
||||||
|
* events), but unconditionally flipping `isLoading` back to `true` for it
|
||||||
|
* hides the ENTIRE conversation — messages, composer, everything — behind a
|
||||||
|
* spinner for as long as the redundant fetch takes. Meanwhile SSE keeps
|
||||||
|
* delivering `message.updated`/`message.part.updated` events the whole
|
||||||
|
* time — the store keeps them, but the screen can't show them while
|
||||||
|
* `isLoading` is blocking the message list. If that redundant fetch is slow
|
||||||
|
* or stalls (flaky mobile network), the conversation looks permanently stuck
|
||||||
|
* "loading" until the user backs out to the sessions list and re-enters —
|
||||||
|
* which works only because it's a fresh attempt that (usually) doesn't hit
|
||||||
|
* the same stall, not because anything was actually fixed.
|
||||||
|
*
|
||||||
|
* A cold load (no session shown yet, or a genuinely different session) still
|
||||||
|
* needs the spinner — there's nothing to show meanwhile. A refresh of the
|
||||||
|
* session already on screen does not: keep showing what's there (which
|
||||||
|
* live SSE updates keep current) while the refetch runs quietly in the
|
||||||
|
* background.
|
||||||
|
*/
|
||||||
|
export function isColdSessionLoad(currentSessionID: string | null | undefined, targetSessionID: string): boolean {
|
||||||
|
return currentSessionID !== targetSessionID
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Decide whether an incoming SSE event's session matches the session a
|
||||||
|
* screen/store is currently bound to — the shared "is this event for me"
|
||||||
|
* check used to key live updates (messages, parts) to the active session,
|
||||||
|
* instead of e.g. matching on stale or missing ids.
|
||||||
|
*/
|
||||||
|
export function isLiveEventForSession(
|
||||||
|
eventSessionID: string | null | undefined,
|
||||||
|
activeSessionID: string | null | undefined,
|
||||||
|
): boolean {
|
||||||
|
return !!eventSessionID && !!activeSessionID && eventSessionID === activeSessionID
|
||||||
|
}
|
||||||
@@ -6,6 +6,7 @@ import { addBreadcrumb } from "../lib/sentry"
|
|||||||
import { AnalyticsEvent, track } from "../lib/analytics"
|
import { AnalyticsEvent, track } from "../lib/analytics"
|
||||||
import { extractPromptFromParts, type PromptFromParts } from "../lib/prompt-from-parts"
|
import { extractPromptFromParts, type PromptFromParts } from "../lib/prompt-from-parts"
|
||||||
import { mergeIncomingMessage } from "../lib/message-merge"
|
import { mergeIncomingMessage } from "../lib/message-merge"
|
||||||
|
import { isColdSessionLoad, isLiveEventForSession } from "../lib/session-load-reconcile"
|
||||||
|
|
||||||
// Helper to convert API response to our internal format
|
// Helper to convert API response to our internal format
|
||||||
function parseMessages(response: MessageWithParts[]): { messages: Message[]; parts: Record<string, Part[]> } {
|
function parseMessages(response: MessageWithParts[]): { messages: Message[]; parts: Record<string, Part[]> } {
|
||||||
@@ -128,10 +129,21 @@ export const useSessions = create<SessionsState>((set, get) => ({
|
|||||||
|
|
||||||
const seq = ++selectSeq
|
const seq = ++selectSeq
|
||||||
addBreadcrumb({ category: "session", message: "select", data: { sessionID, hasDirectory: Boolean(directory) } })
|
addBreadcrumb({ category: "session", message: "select", data: { sessionID, hasDirectory: Boolean(directory) } })
|
||||||
|
// Re-selecting the session already shown on screen (e.g. #121's
|
||||||
|
// useFocusEffect resync firing again on re-entry) is a background
|
||||||
|
// refresh, not a cold load: the screen already has this session's
|
||||||
|
// messages, and live SSE updates keep flowing to them the whole time.
|
||||||
|
// Forcing isLoading back to true here would hide the entire
|
||||||
|
// conversation — including anything streaming in live right now —
|
||||||
|
// behind a spinner for as long as this redundant fetch takes, and if it
|
||||||
|
// stalls (flaky network), the screen looks permanently stuck "loading"
|
||||||
|
// until the user backs out and re-enters (issue #150). Only a
|
||||||
|
// genuinely new/different session needs the blocking spinner.
|
||||||
|
const isColdLoad = isColdSessionLoad(get().currentSession?.id, sessionID)
|
||||||
try {
|
try {
|
||||||
// Reset optimistic sending — SSE sessionStatus is the source of truth
|
// Reset optimistic sending — SSE sessionStatus is the source of truth
|
||||||
set((state) => ({
|
set((state) => ({
|
||||||
isLoading: true,
|
isLoading: isColdLoad ? true : state.isLoading,
|
||||||
error: null,
|
error: null,
|
||||||
hasMore: false,
|
hasMore: false,
|
||||||
loadingMore: false,
|
loadingMore: false,
|
||||||
@@ -409,9 +421,16 @@ export const useSessions = create<SessionsState>((set, get) => ({
|
|||||||
switch (event.type) {
|
switch (event.type) {
|
||||||
case "message.updated": {
|
case "message.updated": {
|
||||||
const message = (props.info || props.message) as Message | undefined
|
const message = (props.info || props.message) as Message | undefined
|
||||||
if (!message || message.sessionID !== currentSession.id) return
|
if (!message || !isLiveEventForSession(message.sessionID, currentSession.id)) return
|
||||||
|
|
||||||
set((state) => ({ messages: mergeIncomingMessage(state.messages, message) }))
|
set((state) => ({
|
||||||
|
messages: mergeIncomingMessage(state.messages, message),
|
||||||
|
// A live update for the session on screen is proof it has content
|
||||||
|
// to show — clear any stuck spinner even if the initial (or a
|
||||||
|
// redundant re-focus) GET hasn't resolved yet, or never does
|
||||||
|
// (issue #150). Only ever clears, never sets it back to true.
|
||||||
|
isLoading: false,
|
||||||
|
}))
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -431,6 +450,9 @@ export const useSessions = create<SessionsState>((set, get) => ({
|
|||||||
? messageParts.map((p) => (p.id === part.id ? part : p))
|
? messageParts.map((p) => (p.id === part.id ? part : p))
|
||||||
: [...messageParts, part],
|
: [...messageParts, part],
|
||||||
},
|
},
|
||||||
|
// See message.updated above — a live part update is just as
|
||||||
|
// much proof of life as a message update.
|
||||||
|
isLoading: false,
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
break
|
break
|
||||||
@@ -453,6 +475,7 @@ export const useSessions = create<SessionsState>((set, get) => ({
|
|||||||
set((state) => ({
|
set((state) => ({
|
||||||
sessions: state.sessions.map((s) => (s.id === session.id ? session : s)),
|
sessions: state.sessions.map((s) => (s.id === session.id ? session : s)),
|
||||||
currentSession: state.currentSession?.id === session.id ? session : state.currentSession,
|
currentSession: state.currentSession?.id === session.id ? session : state.currentSession,
|
||||||
|
isLoading: isLiveEventForSession(session.id, state.currentSession?.id) ? false : state.isLoading,
|
||||||
}))
|
}))
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user