Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions src/lib/session-load-reconcile.test.ts
Original file line number Diff line number Diff line change
@@ -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 changes: 44 additions & 0 deletions src/lib/session-load-reconcile.ts
Original file line number Diff line number Diff line change
@@ -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
}
29 changes: 26 additions & 3 deletions src/stores/sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { addBreadcrumb } from "../lib/sentry"
import { AnalyticsEvent, track } from "../lib/analytics"
import { extractPromptFromParts, type PromptFromParts } from "../lib/prompt-from-parts"
import { mergeIncomingMessage } from "../lib/message-merge"
import { isColdSessionLoad, isLiveEventForSession } from "../lib/session-load-reconcile"

// Helper to convert API response to our internal format
function parseMessages(response: MessageWithParts[]): { messages: Message[]; parts: Record<string, Part[]> } {
Expand Down Expand Up @@ -128,10 +129,21 @@ export const useSessions = create<SessionsState>((set, get) => ({

const seq = ++selectSeq
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 {
// Reset optimistic sending — SSE sessionStatus is the source of truth
set((state) => ({
isLoading: true,
isLoading: isColdLoad ? true : state.isLoading,
error: null,
hasMore: false,
loadingMore: false,
Expand Down Expand Up @@ -409,9 +421,16 @@ export const useSessions = create<SessionsState>((set, get) => ({
switch (event.type) {
case "message.updated": {
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
}

Expand All @@ -431,6 +450,9 @@ export const useSessions = create<SessionsState>((set, get) => ({
? messageParts.map((p) => (p.id === part.id ? part : p))
: [...messageParts, part],
},
// See message.updated above — a live part update is just as
// much proof of life as a message update.
isLoading: false,
}
})
break
Expand All @@ -453,6 +475,7 @@ export const useSessions = create<SessionsState>((set, get) => ({
set((state) => ({
sessions: state.sessions.map((s) => (s.id === session.id ? session : s)),
currentSession: state.currentSession?.id === session.id ? session : state.currentSession,
isLoading: isLiveEventForSession(session.id, state.currentSession?.id) ? false : state.isLoading,
}))
break
}
Expand Down
Loading