85a7f9f · opencode-android

git.iko.soy/opencode-android

Fix four UI/streaming/SSE bugs found by a codebase bug hunt

- ChatScreen: streaming auto-scroll and the jump-to-latest FAB never engaged because the keyless derivedStateOf / LaunchedEffect(Unit) captured the first (empty) listItems forever; read it via rememberUpdatedState so the long-lived lambdas track the live list. - EventStreamClient: an abnormal SSE socket close (TCP reset) threw inside the incoming.produceIn(scope) child, cancelling the whole channelFlow past the retry catch, killing reconnect and crashing via the handler-less shareIn scope. Bridge incoming through our own channel closed with the cause so the failure flows into the existing retry/backoff path. - TwoPaneSessionChat: wrap ChatScreen in key(sessionId) so switching sessions resets position-anchored composition state (scroll, initial-scroll flag, error/retry wiring) instead of leaking it across conversations. - ChatScreen: date-separator LazyColumn keys were label-only and could collide for non-contiguous same-label groups; key on a per-occurrence ordinal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

commit85a7f9fe3d80470ad5033550505d7a97a8444492 authoriko <mail@iko.soy> dateThu 2 Jul 2026, 04:17 (3 months ago) parent2811bd0

3 files changed+79−19

compare with master

unifiedsplit

app/src/main/java/soy/iko/opencode/data/network/EventStreamClient.kt+38−5 app/src/main/java/soy/iko/opencode/ui/chat/ChatScreen.kt+25−8 app/src/main/java/soy/iko/opencode/ui/session/TwoPaneSessionChat.kt+16−6

app/src/main/java/soy/iko/opencode/data/network/EventStreamClient.kt+38−5

@@ -21,9 +21,9 @@ import kotlinx.coroutines.flow.SharingStarted

2121import kotlinx.coroutines.flow.asStateFlow

2222import kotlinx.coroutines.flow.buffer

2323import kotlinx.coroutines.flow.channelFlow

24−import kotlinx.coroutines.flow.produceIn

2524import kotlinx.coroutines.flow.shareIn

2625import kotlinx.coroutines.isActive

26+import kotlinx.coroutines.launch

2727import kotlinx.coroutines.selects.onTimeout

2828import kotlinx.coroutines.selects.select

2929import java.io.IOException

@@ -108,7 +108,15 @@ open class EventStreamClient(

108108 onTimeout(idleTimeoutMs) { null }

109109 }

110110 if (result == null) throw IOException("SSE idle timeout, reconnecting")

111− val sse = result.getOrNull() ?: break

111+ val sse = result.getOrNull()

112+ if (sse == null) {

113+ // The producer channel is closed. A non-null cause means `incoming`

114+ // failed (e.g. a socket reset bridged from the producer) — rethrow it so

115+ // the outer loop logs and retries with backoff. A clean close (null

116+ // cause) just ends this attempt and lets the loop reconnect.

117+ result.exceptionOrNull()?.let { throw it }

118+ break

119+ }

112120 val data = sse.data

113121 if (data == null) {

114122 // A comment or keep-alive event (no data field) resets the idle watchdog

@@ -169,9 +177,33 @@ open class EventStreamClient(

169177 // made while the previous stream was healthy, so it doesn't suppress

170178 // the next legitimate backoff.

171179 while (reconnectSignal.tryReceive().isSuccess) { /* drain */ }

172− // `incoming` is a cold Flow; bridge it to a ReceiveChannel so each

173− // element can be raced against an idle timeout via select.

174− val events = incoming.produceIn(scope)

180+ // Bridge ktor's `incoming` flow to a channel so each element can be

181+ // raced against the idle-timeout watchdog via select. We collect it

182+ // ourselves instead of `incoming.produceIn(scope)`: produceIn launches

183+ // the collector as a child of this channelFlow, so when `incoming`

184+ // throws on an abnormal socket close — a TCP reset, i.e. the common

185+ // mobile case: Wi-Fi⇄cellular handoff, server or proxy RST — the child's

186+ // failure cancels the whole channelFlow and bypasses the retry `catch`

187+ // below. That killed reconnect outright and surfaced the IOException as

188+ // an uncaught crash through the (handler-less) shareIn scope. Collecting

189+ // into our own channel and closing it with the cause keeps the failure

190+ // local: readSseEvents sees the closed channel, rethrows the cause, and

191+ // the normal retry/backoff path handles it. (A clean EOF closes the

192+ // channel with no cause → readSseEvents breaks and reconnects as before.)

193+ val events = Channel<ServerSentEvent>(Channel.BUFFERED)

194+ val producer = scope.launch {

195+ try {

196+ incoming.collect { events.send(it) }

197+ events.close()

198+ } catch (c: CancellationException) {

199+ events.close(c)

200+ throw c

201+ } catch (t: Throwable) {

202+ // Deliver the failure via the channel rather than letting it

203+ // propagate up and cancel the parent channelFlow.

204+ events.close(t)

205+ }

206+ }

175207 // Reset the backoff only once real data actually arrives, not on

176208 // block entry: a server that accepts the connection then immediately

177209 // drops it would otherwise loop forever at the initial backoff and

@@ -186,6 +218,7 @@ open class EventStreamClient(

186218 send(it)

187219 }

188220 } finally {

221+ producer.cancel()

189222 events.cancel()

190223 }

191224 }

app/src/main/java/soy/iko/opencode/ui/chat/ChatScreen.kt+25−8

@@ -61,6 +61,7 @@ import androidx.compose.runtime.getValue

6161import androidx.compose.runtime.mutableStateOf

6262import androidx.compose.runtime.remember

6363import androidx.compose.runtime.rememberCoroutineScope

64+import androidx.compose.runtime.rememberUpdatedState

6465import androidx.compose.runtime.saveable.rememberSaveable

6566import androidx.compose.runtime.setValue

6667import androidx.compose.runtime.snapshotFlow

@@ -440,6 +441,15 @@ fun ChatScreen(

440441 val listItems = remember(messages, todayLabel, yesterdayLabel) {

441442 buildMessageListItems(messages, todayLabel, yesterdayLabel)

442443 }

444+ // listItems is a plain (non-snapshot) local, rebuilt each recomposition. The

445+ // derivedStateOf and snapshotFlow lambdas below are created once (keyless

446+ // remember / LaunchedEffect(Unit)); reading `listItems` directly inside them

447+ // would capture the first composition's list — which is empty, since `messages`

448+ // starts empty — freezing it forever. That pinned isPinnedToBottom at true and

449+ // AutoScrollSignal.size at 0, breaking streaming auto-scroll and the

450+ // jump-to-latest FAB. rememberUpdatedState hands those lambdas a stable State

451+ // whose value tracks the latest list.

452+ val currentListItems by rememberUpdatedState(listItems)

443453

444454 val isPinnedToBottom by remember {

445455 derivedStateOf {

@@ -449,8 +459,9 @@ fun ChatScreen(

449459 // includes the working indicator — otherwise the pin check targets

450460 // listItems.lastIndex, the typing row sits just below the viewport,

451461 // and the user never sees the progress indicator even when pinned.

452− val effectiveLast = if (running) listItems.size else listItems.lastIndex

453− lastVisible >= effectiveLast || listItems.isEmpty()

462+ val items = currentListItems

463+ val effectiveLast = if (running) items.size else items.lastIndex

464+ lastVisible >= effectiveLast || items.isEmpty()

454465 }

455466 }

456467

@@ -495,13 +506,14 @@ fun ChatScreen(

495506 // and tool output — not just TextPart — keeps a pinned view following a long

496507 // reasoning/tool block while it streams (previously it stalled at 0).

497508 val lastLen = streamingContentLength(messages.lastOrNull()?.parts?.lastOrNull())

498− AutoScrollSignal(listItems.size, lastLen, isPinnedToBottom)

509+ AutoScrollSignal(currentListItems.size, lastLen, isPinnedToBottom)

499510 }.collect { signal ->

500511 if (signal.size > 0 && signal.pinned) {

501512 // Scroll to the effective last index, including the trailing

502513 // "__typing" row when a run is active so the working indicator

503514 // is brought into view (not just the last message).

504− val target = if (running) listItems.size else listItems.lastIndex

515+ val items = currentListItems

516+ val target = if (running) items.size else items.lastIndex

505517 listState.scrollToItem(target)

506518 }

507519 }

@@ -1056,9 +1068,13 @@ private sealed interface MessageListItem {

10561068 override val contentType: Any get() = message.info::class

10571069 }

10581070

1059− data class Separator(val label: String) : MessageListItem {

1060− // Stable key per label so a recomposition doesn't re-create the item.

1061− override val key: Any get() = "sep_$label"

1071+ data class Separator(val label: String, val ordinal: Int) : MessageListItem {

1072+ // Key on the day-occurrence ordinal, not the label: two non-contiguous groups

1073+ // can share a label (e.g. an untimestamped message — empty label — between two

1074+ // same-day messages, or two untimestamped groups), and a label-only key would

1075+ // then collide, which makes LazyColumn throw on duplicate keys. The ordinal is

1076+ // stable for a given message ordering so slots aren't needlessly recreated.

1077+ override val key: Any get() = "sep_${ordinal}_$label"

10621078 // Separators share a contentType so the LazyColumn can recycle their slots.

10631079 override val contentType: Any get() = "separator"

10641080 }

@@ -1085,11 +1101,12 @@ private fun buildMessageListItems(

10851101 val dateFmt = java.text.DateFormat.getDateInstance(java.text.DateFormat.MEDIUM)

10861102 val result = ArrayList<MessageListItem>(messages.size + 4)

10871103 var lastDayKey: String? = null

1104+ var sepOrdinal = 0

10881105 for (message in messages) {

10891106 val ts = message.info.time?.created ?: message.info.time?.updated ?: message.info.time?.completed

10901107 val dayKey = ts?.let { dayKey(it) } ?: ""

10911108 if (dayKey != lastDayKey) {

1092− result.add(MessageListItem.Separator(dayLabel(dayKey, ts ?: 0L, today, todayLabel, yesterdayLabel, dateFmt)))

1109+ result.add(MessageListItem.Separator(dayLabel(dayKey, ts ?: 0L, today, todayLabel, yesterdayLabel, dateFmt), sepOrdinal++))

10931110 lastDayKey = dayKey

10941111 }

10951112 result.add(MessageListItem.Message(message))

app/src/main/java/soy/iko/opencode/ui/session/TwoPaneSessionChat.kt+16−6

@@ -16,6 +16,7 @@ import androidx.compose.material3.Text

1616import androidx.compose.runtime.Composable

1717import androidx.compose.runtime.LaunchedEffect

1818import androidx.compose.runtime.getValue

19+import androidx.compose.runtime.key

1920import androidx.compose.runtime.mutableStateOf

2021import androidx.compose.runtime.remember

2122import androidx.compose.runtime.saveable.rememberSaveable

@@ -114,12 +115,21 @@ fun TwoPaneSessionChat(

114115 EmptyDetail()

115116 } else {

116117 BackHandler { selected = null }

117− ChatScreen(

118− container = container,

119− sessionId = sessionId,

120− onBack = { selected = null },

121− onOpenFile = onOpenFile,

122− )

118+ // key() on the session id so switching conversations in two-pane mode

119+ // gives ChatScreen a fresh composition. Without it, position-anchored

120+ // state (rememberLazyListState, didInitialScroll, and the LaunchedEffect(Unit)

121+ // that wires error/retry snackbars) persists across the switch: the new

122+ // session inherits the old scroll offset, skips its initial scroll-to-bottom,

123+ // and its error events go unhandled. viewModel(key=sessionId) already swaps

124+ // the VM; this aligns the composition state with it.

125+ key(sessionId) {

126+ ChatScreen(

127+ container = container,

128+ sessionId = sessionId,

129+ onBack = { selected = null },

130+ onOpenFile = onOpenFile,

131+ )

132+ }

123133 }

124134 }

125135 }