From 82e2d787af04d8a5b4fda35776dd74b5b8dcc055 Mon Sep 17 00:00:00 2001 From: M57 <41645758+Hy4ri@users.noreply.github.com> Date: Tue, 18 Aug 2026 17:06:51 +0300 Subject: [PATCH] fix(sessions): show fork icon + order number on nested forked sessions (#934) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(sessions): fork icon + depth number, no indentation Multiply-nested forked sessions rendered with deep indentation that crushed the card off-screen and made titles unreadable, and the per-sibling order number was uninformative (mostly "1"). - SessionTreeItem: replace forkIndex (sibling order) with forkDepth (nesting level: 1 = child of a root, 2 = fork of a fork, ...); flattenSessionTree populates it. - SessionCard: render CallSplit fork icon + the depth number (primary tint). Removed the depth-based start padding so names stay full-width readable — the icon+number now carries the structure instead of indentation. - strings: sessions_fork_indicator (en/zh/ko). - SessionTreeTest: assert isFork/forkDepth across nested fixtures. No backend change — backend already returns parent_session_id per SessionInfo. * test(ws): make testAutoReconnect tolerate transient RECONNECTING state The test gated on connectionStatus.first { it == RECONNECTING } with a 15s timeout. With the test backoff forced to 0ms the RECONNECTING state is emitted + consumed before the collector attaches, so the wait missed it and timed out (kotlinx.coroutines.TimeoutCancellationException) — the flake CI saw under parallel load. - Replace the brittle RECONNECTING .first{} wait with a tolerant check that accepts RECONNECTING-or-later (the reconnect clearly fired). - Keep connect2Latch (second socket opening) as the definitive proof + add a final CONNECTED assertion. - Reset the test backoff in a finally so later tests never inherit 0ms. --------- Co-authored-by: M57 --- .../data/model/SessionTreeItem.kt | 35 ++++++++++++++++--- .../ui/sessions/SessionsScreen.kt | 32 ++++++++++++----- app/src/main/res/values-ko/strings.xml | 1 + app/src/main/res/values-zh/strings.xml | 1 + app/src/main/res/values/strings.xml | 1 + .../data/model/SessionTreeTest.kt | 24 ++++++++++++- .../data/ws/HermesWsClientTest.kt | 35 +++++++++++++------ 7 files changed, 103 insertions(+), 26 deletions(-) diff --git a/app/src/main/java/com/m57/hermescontrol/data/model/SessionTreeItem.kt b/app/src/main/java/com/m57/hermescontrol/data/model/SessionTreeItem.kt index f5cccdc4..4890e064 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/model/SessionTreeItem.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/model/SessionTreeItem.kt @@ -4,9 +4,25 @@ data class SessionTreeItem( val session: SessionInfo, val depth: Int, val branchStem: String?, + // True when this session was forked/branched from another (has a parent in + // the loaded set). Drives the fork icon in SessionCard. + val isFork: Boolean = false, + // How many levels deep this fork sits (1 = direct child of a root, 2 = fork + // of a fork, ...). Shown as the number next to the fork icon so a nested + // chain reads ðŸī1 → ðŸī2 → ðŸī3 and a branch-off pops out visually. 0 for + // non-forked (root) sessions. + val forkDepth: Int = 0, val displayTitle: String, ) +/** + * Flatten a session list into a display-order tree. + * + * Branching is expressed by `parent_session_id`. A session is a "fork" when its + * parent is present in the set; `forkDepth` is its nesting depth (1 = child of a + * root). The caller (SessionCard) renders a fork icon + the depth number so + * multiply-nested forks stay readable without deep indentation. + */ fun flattenSessionTree(sessions: List): List { val ids = sessions.mapTo(mutableSetOf()) { it.id } val children = sessions.groupBy { it.parent_session_id?.takeIf(ids::contains) } @@ -16,8 +32,8 @@ fun flattenSessionTree(sessions: List): List { fun append( session: SessionInfo, depth: Int, - branchStem: String?, siblingIndex: Int, + siblingCount: Int, ) { if (!visited.add(session.id)) return val isBranch = session.parent_session_id?.takeIf(ids::contains) != null @@ -29,23 +45,32 @@ fun flattenSessionTree(sessions: List): List { } else { session.preview?.takeIf(String::isNotBlank)?.take(80) ?: "Untitled" } - result += SessionTreeItem(session, depth, branchStem, title) + val branchStem = if (depth == 0) null else (if (siblingIndex == siblingCount - 1) "└─" else "├─") + result += + SessionTreeItem( + session = session, + depth = depth, + branchStem = branchStem, + isFork = isBranch, + forkDepth = if (isBranch) depth else 0, + displayTitle = title, + ) val descendants = children[session.id].orEmpty() descendants.forEachIndexed { index, child -> append( session = child, depth = depth + 1, - branchStem = if (index == descendants.lastIndex) "└─" else "├─", siblingIndex = index, + siblingCount = descendants.size, ) } } children[null].orEmpty().forEachIndexed { index, root -> - append(root, depth = 0, branchStem = null, siblingIndex = index) + append(root, depth = 0, siblingIndex = index, siblingCount = children[null].orEmpty().size) } sessions.filterNot { it.id in visited }.forEachIndexed { index, orphan -> - append(orphan, depth = 0, branchStem = null, siblingIndex = index) + append(orphan, depth = 0, siblingIndex = index, siblingCount = 1) } return result } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/sessions/SessionsScreen.kt b/app/src/main/java/com/m57/hermescontrol/ui/sessions/SessionsScreen.kt index e18e19fc..f37abe35 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/sessions/SessionsScreen.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/sessions/SessionsScreen.kt @@ -32,6 +32,7 @@ import androidx.compose.foundation.text.KeyboardActions import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.Send +import androidx.compose.material.icons.filled.CallSplit import androidx.compose.material.icons.filled.CheckCircle import androidx.compose.material.icons.filled.Close import androidx.compose.material.icons.filled.Code @@ -791,8 +792,9 @@ fun SessionsScreen( SessionCard( session = session, displayTitle = item.displayTitle, - depth = item.depth, branchStem = item.branchStem, + isFork = item.isFork, + forkDepth = item.forkDepth, query = state.searchQuery, isSelecting = state.isSelecting, isSelected = session.id in state.selectedIds, @@ -958,8 +960,9 @@ fun SessionsScreen( private fun SessionCard( session: com.m57.hermescontrol.data.model.SessionInfo, displayTitle: String, - depth: Int, branchStem: String?, + isFork: Boolean = false, + forkDepth: Int = 0, query: String, isSelecting: Boolean, isSelected: Boolean, @@ -984,7 +987,6 @@ private fun SessionCard( modifier = Modifier .fillMaxWidth() - .padding(start = (depth * 16).dp) .testTag("session_card_${session.id}") .combinedClickable( onClick = onCardClick, @@ -1022,13 +1024,25 @@ private fun SessionCard( Spacer(modifier = Modifier.width(spacing.sm)) } - // Branch tree stem - if (branchStem != null && !isSelecting) { - Text( - text = branchStem, - style = MaterialTheme.typography.labelLarge, - color = MaterialTheme.colorScheme.outline, + // Fork marker: a fork icon + its nesting depth, so a chain reads + // ðŸī1 → ðŸī2 → ðŸī3 and a branch-off pops out. No indentation — the + // number carries the structure so names never get squished. + if (isFork && !isSelecting) { + Icon( + imageVector = Icons.Filled.CallSplit, + contentDescription = stringResource(R.string.sessions_fork_indicator), + tint = MaterialTheme.colorScheme.primary, + modifier = Modifier.size(16.dp), ) + if (forkDepth > 0) { + Spacer(modifier = Modifier.width(spacing.xs)) + Text( + text = forkDepth.toString(), + style = MaterialTheme.typography.labelLarge, + fontWeight = FontWeight.Bold, + color = MaterialTheme.colorScheme.primary, + ) + } Spacer(modifier = Modifier.width(spacing.sm)) } diff --git a/app/src/main/res/values-ko/strings.xml b/app/src/main/res/values-ko/strings.xml index 0fc2c695..b8ba7493 100644 --- a/app/src/main/res/values-ko/strings.xml +++ b/app/src/main/res/values-ko/strings.xml @@ -347,6 +347,7 @@ ė§€ė •ëœ ėžėˆ˜ëģīë‹Ī ė˜Ī래된 ė„ļė…˜ė„ ė‚­ė œí•Đ니ë‹Ī. ėžėˆ˜ ęļ°ėĪ€ ė •ëĶŽ + ëķ„ęļ°ëœ ė„ļė…˜ ė„ļė…˜ ė‚­ė œ? ė •ë§ \"%1$s\"ė„(ëĨž) ė‚­ė œí•˜ė‹œęē ėŠĩ니ęđŒ? 되돌ëĶī ėˆ˜ ė—†ėŠĩ니ë‹Ī. ė„ļė…˜ ė‚­ė œ? diff --git a/app/src/main/res/values-zh/strings.xml b/app/src/main/res/values-zh/strings.xml index 67626a45..3c604ad5 100644 --- a/app/src/main/res/values-zh/strings.xml +++ b/app/src/main/res/values-zh/strings.xml @@ -473,6 +473,7 @@ į―ŪéĄķ 取æķˆį―ŪéĄķ å·ēį―ŪéĄķ + 分æ”Ŋ侚čŊ é‡å‘―åäžščŊ 侚čŊæ ‡éĒ˜ æēĄæœ‰å·Ĩ具集数æŪ diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 968d28ed..4141c873 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -548,6 +548,7 @@ Pin Unpin Pinned + Forked session Rename Session Session title diff --git a/app/src/test/java/com/m57/hermescontrol/data/model/SessionTreeTest.kt b/app/src/test/java/com/m57/hermescontrol/data/model/SessionTreeTest.kt index bfc28b16..38bdd318 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/model/SessionTreeTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/model/SessionTreeTest.kt @@ -25,7 +25,29 @@ class SessionTreeTest { it.displayTitle }, ) - assertEquals(listOf(null, null, "├─", "└─", "└─"), tree.map { it.branchStem }) + // Fork flag + nesting depth (1 = child of root, 2 = fork of a fork). + assertEquals(listOf(false, false, true, true, true), tree.map { it.isFork }) + assertEquals(listOf(0, 0, 1, 1, 2), tree.map { it.forkDepth }) + } + + @Test + fun deepNestingMarksEveryForkWithDepth() { + // root → a (child 1) → a1 (child 1) ; root → b (child 2, last) + // a1 is fork-depth 2 (fork of a fork), b is fork-depth 1. + val sessions = + listOf( + SessionInfo(id = "root", title = "Root"), + SessionInfo(id = "a", parent_session_id = "root"), + SessionInfo(id = "b", parent_session_id = "root"), + SessionInfo(id = "a1", parent_session_id = "a"), + ) + + val tree = flattenSessionTree(sessions) + + assertEquals(listOf("root", "a", "a1", "b"), tree.map { it.session.id }) + assertEquals(listOf(0, 1, 2, 1), tree.map { it.depth }) + assertEquals(listOf(false, true, true, true), tree.map { it.isFork }) + assertEquals(listOf(0, 1, 2, 1), tree.map { it.forkDepth }) } @Test diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt index 40ca4762..c219109b 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt @@ -1099,20 +1099,33 @@ class HermesWsClientTest { // Force server to close socket 1 to trigger reconnect serverSocket1?.close(1001, "Server shutting down") - // Wait for status to become RECONNECTING - runBlocking { - withTimeout( - 15_000, - ) { HermesWsClient.connectionStatus.first { it == ConnectionStatus.RECONNECTING } } - } + // Wait for the reconnect to trigger. With the test backoff forced to + // 0ms the RECONNECTING state is transient and can be emitted + consumed + // before this collector attaches, so we don't gate on the exact + // intermediate state — we require it to have progressed (RECONNECTING + // or later). The definitive proof is the second socket opening below. + val statusNow = HermesWsClient.connectionStatus.value + assertTrue( + "Expected reconnect to trigger (RECONNECTING or later), was $statusNow", + statusNow == ConnectionStatus.RECONNECTING || + statusNow == ConnectionStatus.CONNECTING || + statusNow == ConnectionStatus.CONNECTED, + ) - // The client should now attempt to reconnect after initial backoff (1000ms) + // The client should now attempt to reconnect after the (0ms) backoff. // Wait for the second connection to hit the server. Generous ceiling: // loaded CI runners routinely stretch short wall-clock windows. - assertTrue("Failed to reconnect", connect2Latch.await(30, TimeUnit.SECONDS)) - - // Restore production backoff so later tests see the default. - HermesWsClient.setReconnectBackoffForTest(1_000L) + try { + assertTrue("Failed to reconnect", connect2Latch.await(30, TimeUnit.SECONDS)) + runBlocking { + withTimeout(15_000) { HermesWsClient.connectionStatus.first { it == ConnectionStatus.CONNECTED } } + } + assertEquals(ConnectionStatus.CONNECTED, HermesWsClient.connectionStatus.value) + } finally { + // Restore production backoff so later tests see the default, even + // if an assertion above threw. + HermesWsClient.setReconnectBackoffForTest(1_000L) + } } // ── TEST-10: WS reconnect state recovery ────────────────────────────