git.iko.soy/opencode-android
on master help sign in
Fix markdown streaming throttle starvation and server edit state races
598c306iko
app/src/main/java/soy/iko/opencode/ui/components/MarkdownText.kt+50−17
compare with master
unifiedsplit
@@ -26,7 +26,9 @@ import androidx.compose.runtime.LaunchedEffect
26import androidx.compose.runtime.getValue
26import androidx.compose.runtime.getValue
27import androidx.compose.runtime.mutableStateOf
27import androidx.compose.runtime.mutableStateOf
28import androidx.compose.runtime.remember
28import androidx.compose.runtime.remember
29+import androidx.compose.runtime.rememberUpdatedState
29import androidx.compose.runtime.setValue
30import androidx.compose.runtime.setValue
31+import androidx.compose.runtime.snapshotFlow
30import androidx.compose.ui.Alignment
32import androidx.compose.ui.Alignment
31import androidx.compose.ui.Modifier
33import androidx.compose.ui.Modifier
32import androidx.compose.ui.draw.clip
34import androidx.compose.ui.draw.clip
@@ -35,6 +37,7 @@ import androidx.compose.ui.semantics.Role
35import androidx.compose.ui.text.TextStyle
37import androidx.compose.ui.text.TextStyle
36import androidx.compose.ui.text.font.FontFamily
38import androidx.compose.ui.text.font.FontFamily
37import androidx.compose.ui.unit.dp
39import androidx.compose.ui.unit.dp
40+import kotlinx.coroutines.flow.conflate
38import kotlinx.coroutines.delay
41import kotlinx.coroutines.delay
39import com.mikepenz.markdown.compose.components.MarkdownComponentModel
42import com.mikepenz.markdown.compose.components.MarkdownComponentModel
40import com.mikepenz.markdown.compose.components.markdownComponents
43import com.mikepenz.markdown.compose.components.markdownComponents
@@ -55,6 +58,14 @@ import soy.iko.opencode.data.network.NetworkConfig
55 * whenever the content string changes). To avoid O(n²) work during long responses, the
58 * whenever the content string changes). To avoid O(n²) work during long responses, the
56 * rendered content is throttled — the latest [markdown] is committed to the renderer at
59 * rendered content is throttled — the latest [markdown] is committed to the renderer at
57 * most once per frame (~16ms), so a burst of tokens coalesces into a single re-parse.
60 * most once per frame (~16ms), so a burst of tokens coalesces into a single re-parse.
61+ *
62+ * The throttle uses a single long-lived `LaunchedEffect(Unit)` that observes the markdown
63+ * parameter via `rememberUpdatedState` + `snapshotFlow` + `conflate` + `collect`. A keyed
64+ * effect (`LaunchedEffect(markdown)`) cancels and restarts on every token; if tokens arrive
65+ * faster than the throttle delay, the in-flight `delay` is cancelled before it completes,
66+ * starving the render and showing stale content until the stream pauses. The conflated
67+ * `collect` lets the delay run to completion, then picks up the most recent buffered value,
68+ * so rendering always progresses even under continuous fast streaming.
58 */
69 */
59@OptIn(ExperimentalFoundationApi::class)
70@OptIn(ExperimentalFoundationApi::class)
60@Composable
71@Composable
@@ -64,21 +75,29 @@ fun MarkdownText(
64 style: TextStyle = MaterialTheme.typography.bodyLarge,
75 style: TextStyle = MaterialTheme.typography.bodyLarge,
65) {
76) {
66 val context = LocalContext.current
77 val context = LocalContext.current
67− // Throttle the rendered content so a rapid burst of streaming tokens coalesces
78+ // Bridge the markdown parameter into snapshot state so snapshotFlow can observe it.
68− // into a single markdown re-parse per frame instead of one per token. The delay
79+ val markdownState = rememberUpdatedState(markdown)
69− // is only applied when the content is growing incrementally (streaming) — a
70− // content switch or initial render proceeds immediately.
71 // Keyed to a content-prefix so switching to a different message resets
80 // Keyed to a content-prefix so switching to a different message resets
72 // immediately instead of showing the old message for one frame.
81 // immediately instead of showing the old message for one frame.
73 var renderedContent by remember(markdown.take(32)) { mutableStateOf(markdown) }
82 var renderedContent by remember(markdown.take(32)) { mutableStateOf(markdown) }
83+ // conflate() coalesces a burst of token updates into a single emission so the
84+ // collector sees only the latest value after the previous delay completes. Plain
85+ // collect (not collectLatest) is critical: collectLatest would cancel the delay
86+ // on every new value, reintroducing the same starvation the keyed effect had.
74− LaunchedEffect(markdown) {
87+ LaunchedEffect(markdown.take(32)) {
88+ snapshotFlow { markdownState.value }
89+ .conflate()
90+ .collect { md ->
91+ // Only throttle when the content is growing incrementally (streaming):
92+ // a content switch or initial render proceeds immediately.
75− if (renderedContent.isNotEmpty() &&
93+ if (renderedContent.isNotEmpty() &&
76− markdown.startsWith(renderedContent) &&
94+ md.startsWith(renderedContent) &&
77− markdown != renderedContent
95+ md != renderedContent
78− ) {
96+ ) {
79− delay(NetworkConfig.streamingThrottleMs)
97+ delay(NetworkConfig.streamingThrottleMs)
80− }
98+ }
81− renderedContent = markdown
99+ renderedContent = md
100+ }
82 }
101 }
83 Markdown(
102 Markdown(
84 content = renderedContent,
103 content = renderedContent,
@@ -101,7 +120,13 @@ fun MarkdownText(
101@Composable
120@Composable
102private fun CodeWithCopy(model: MarkdownComponentModel) {
121private fun CodeWithCopy(model: MarkdownComponentModel) {
103 val context = LocalContext.current
122 val context = LocalContext.current
123+ // Key on the content string + the AST node's offset range (stable ints) instead of
124+ // the model itself. MarkdownComponentModel is not @Immutable/@Stable (it holds an
125+ // ASTNode), so remembering on it re-executes extractCode on every recomposition.
126+ val node = model.node
127+ val code = remember(model.content, node.startOffset, node.endOffset) {
104− val code = androidx.compose.runtime.remember(model) { extractCode(model.content, model.node) }
128+ extractCode(model.content, node)
129+ }
105 val codeStyle = model.typography.code.copy(fontFamily = FontFamily.Monospace)
130 val codeStyle = model.typography.code.copy(fontFamily = FontFamily.Monospace)
106 Box(
131 Box(
107 modifier = Modifier
132 modifier = Modifier
@@ -143,10 +168,18 @@ private fun CodeWithCopy(model: MarkdownComponentModel) {
143 * Extract the raw code text from a markdown code node, stripping fence markers
168 * Extract the raw code text from a markdown code node, stripping fence markers
144 * (```/~~~ and the language tag) so only the code content is copied.
169 * (```/~~~ and the language tag) so only the code content is copied.
145 */
170 */
146−private fun extractCode(content: String, node: ASTNode): String {
171+private fun extractCode(content: String, node: ASTNode): String =
147− val raw = node.getTextInNode(content).toString()
148− val isOpenFence = node.type == MarkdownElementTypes.CODE_FENCE
172+ extractCodeText(node.getTextInNode(content).toString(), node.type == MarkdownElementTypes.CODE_FENCE)
173+
174+/**
175+ * Strip fence markers (```/~~~ and the language tag) from a raw code node text so
176+ * only the code content is returned. Extracted as a pure function for testability.
177+ *
178+ * @param raw the raw text of the code node (including fence markers for fenced blocks)
179+ * @param isFenced true for CODE_FENCE nodes, false for indented CODE_BLOCK nodes
180+ */
181+internal fun extractCodeText(raw: String, isFenced: Boolean): String {
149− if (!isOpenFence) return raw.trimIndent()
182+ if (!isFenced) return raw.trimIndent()
150 val lines = raw.lines()
183 val lines = raw.lines()
151 val body = lines.drop(1).toMutableList()
184 val body = lines.drop(1).toMutableList()
152 if (body.isNotEmpty() && (body.last().startsWith("```") || body.last().startsWith("~~~"))) {
185 if (body.isNotEmpty() && (body.last().startsWith("```") || body.last().startsWith("~~~"))) {
app/src/main/java/soy/iko/opencode/ui/server/ServerEditViewModel.kt+5−4
@@ -10,6 +10,7 @@ import kotlinx.coroutines.flow.MutableStateFlow
10import kotlinx.coroutines.flow.StateFlow
10import kotlinx.coroutines.flow.StateFlow
11import kotlinx.coroutines.flow.asStateFlow
11import kotlinx.coroutines.flow.asStateFlow
12import kotlinx.coroutines.flow.first
12import kotlinx.coroutines.flow.first
13+import kotlinx.coroutines.flow.update
13import kotlinx.coroutines.launch
14import kotlinx.coroutines.launch
14import kotlinx.coroutines.withTimeoutOrNull
15import kotlinx.coroutines.withTimeoutOrNull
15import soy.iko.opencode.R
16import soy.iko.opencode.R
@@ -76,18 +77,18 @@ class ServerEditViewModel(
76 )
77 )
77 return@launch
78 return@launch
78 }
79 }
79− _state.value = _state.value.copy(loaded = true)
80+ _state.update { it.copy(loaded = true) }
80 }
81 }
81 }
82 }
82
83
83 fun update(transform: (ServerEditState) -> ServerEditState) {
84 fun update(transform: (ServerEditState) -> ServerEditState) {
84− _state.value = transform(_state.value)
85+ _state.update(transform)
85 }
86 }
86
87
87 fun save(onDone: () -> Unit) {
88 fun save(onDone: () -> Unit) {
88 val s = _state.value
89 val s = _state.value
89 if (!s.canSave || s.saving) return
90 if (!s.canSave || s.saving) return
90− _state.value = s.copy(saving = true)
91+ _state.update { it.copy(saving = true) }
91 viewModelScope.launch {
92 viewModelScope.launch {
92 val result = runCatchingCancellable {
93 val result = runCatchingCancellable {
93 val existingLastUsed = if (s.id != null) {
94 val existingLastUsed = if (s.id != null) {
@@ -108,7 +109,7 @@ class ServerEditViewModel(
108 )
109 )
109 }
110 }
110 result.onSuccess { onDone() }
111 result.onSuccess { onDone() }
111− .onFailure { _state.value = _state.value.copy(error = container.friendlyError(it), saving = false) }
112+ .onFailure { e -> _state.update { it.copy(error = container.friendlyError(e), saving = false) } }
112 }
113 }
113 }
114 }
114}
115}
app/src/test/java/soy/iko/opencode/ui/components/ExtractCodeTextTest.ktnew file+56
@@ -0,0 +1,56 @@
1+package soy.iko.opencode.ui.components
2+
3+import org.junit.Assert.assertEquals
4+import org.junit.Test
5+
6+class ExtractCodeTextTest {
7+
8+ @Test
9+ fun fencedCodeStripsOpeningAndClosingFence() {
10+ val raw = "```kotlin\nfun main() {}\n```"
11+ assertEquals("fun main() {}", extractCodeText(raw, isFenced = true))
12+ }
13+
14+ @Test
15+ fun fencedCodeWithTildeFenceStripped() {
16+ val raw = "~~~python\nprint('hi')\n~~~"
17+ assertEquals("print('hi')", extractCodeText(raw, isFenced = true))
18+ }
19+
20+ @Test
21+ fun fencedCodePreservesBlankLines() {
22+ val raw = "```text\nline1\n\nline3\n```"
23+ assertEquals("line1\n\nline3", extractCodeText(raw, isFenced = true))
24+ }
25+
26+ @Test
27+ fun fencedCodeWithoutClosingFence() {
28+ // A streaming code block may not have a closing fence yet.
29+ val raw = "```kotlin\nfun main() {}"
30+ assertEquals("fun main() {}", extractCodeText(raw, isFenced = true))
31+ }
32+
33+ @Test
34+ fun fencedEmptyBody() {
35+ val raw = "```kotlin\n```"
36+ assertEquals("", extractCodeText(raw, isFenced = true))
37+ }
38+
39+ @Test
40+ fun fencedMultilineCode() {
41+ val raw = "```js\nconst a = 1;\nconst b = 2;\n```"
42+ assertEquals("const a = 1;\nconst b = 2;", extractCodeText(raw, isFenced = true))
43+ }
44+
45+ @Test
46+ fun indentedCodeTrimIndent() {
47+ val raw = " fun main() {\n println()\n }"
48+ assertEquals("fun main() {\n println()\n}", extractCodeText(raw, isFenced = false))
49+ }
50+
51+ @Test
52+ fun indentedCodeNoIndent() {
53+ val raw = "code without indent"
54+ assertEquals("code without indent", extractCodeText(raw, isFenced = false))
55+ }
56+}
app/src/test/java/soy/iko/opencode/ui/server/ServerEditViewModelTest.kt+30
@@ -227,4 +227,34 @@ class ServerEditViewModelTest {
227 assertNull(saved.username)
227 assertNull(saved.username)
228 assertNull(saved.password)
228 assertNull(saved.password)
229 }
229 }
230+
231+ @Test
232+ fun save_failureSetsErrorAndClearsSaving() = testScope.runTest {
233+ val container = FakeAppContainer()
234+ container.fakeProfileStore.saveException = RuntimeException("disk full")
235+ val vm = makeVm(container)
236+ vm.update { it.copy(baseUrl = "http://localhost:3000") }
237+ var doneCalled = false
238+ vm.save { doneCalled = true }
239+ testScheduler.advanceUntilIdle()
240+ assertFalse(doneCalled)
241+ assertFalse(vm.state.value.saving)
242+ // The error must come from the exception, not from a clobbered ServerEditState.
243+ assertNotNull(vm.state.value.error)
244+ assertTrue(vm.state.value.error!!.contains("disk full"))
245+ }
246+
247+ @Test
248+ fun save_failureDoesNotClobberUserEdits() = testScope.runTest {
249+ val container = FakeAppContainer()
250+ container.fakeProfileStore.saveException = RuntimeException("disk full")
251+ val vm = makeVm(container)
252+ vm.update { it.copy(baseUrl = "http://localhost:3000", label = "Original") }
253+ vm.save { }
254+ // Simulate the user editing the label while the save coroutine is in-flight.
255+ vm.update { it.copy(label = "Edited") }
256+ testScheduler.advanceUntilIdle()
257+ // The error path must not overwrite the user's edit.
258+ assertEquals("Edited", vm.state.value.label)
259+ }
230}
260}
app/src/test/java/soy/iko/opencode/ui/testing/TestFakes.kt+4
@@ -80,9 +80,13 @@ class FakeProfileStore : ProfileStore() {
80 var savedProfile: ServerProfile? = null
80 var savedProfile: ServerProfile? = null
81 private set
81 private set
82
82
83+ /** When non-null, [save] throws this exception instead of persisting. */
84+ var saveException: Exception? = null
85+
83 override suspend fun resolve(profile: ServerProfile): ServerProfile = profile
86 override suspend fun resolve(profile: ServerProfile): ServerProfile = profile
84
87
85 override suspend fun save(profile: ServerProfile) {
88 override suspend fun save(profile: ServerProfile) {
89+ saveException?.let { throw it }
86 savedProfile = profile
90 savedProfile = profile
87 val current = _profiles.value.toMutableList()
91 val current = _profiles.value.toMutableList()
88 current.removeAll { it.id == profile.id }
92 current.removeAll { it.id == profile.id }