From 76726dd0860f86908f03ea005ec3cf13e9a867b2 Mon Sep 17 00:00:00 2001 From: m57 <41645758+Hy4ri@users.noreply.github.com> Date: Mon, 28 Sep 2026 14:30:05 +0300 Subject: [PATCH] refactor(chat): isolate transcript and browser return state (#1320, #1321) (#1357) --- .../hermescontrol/ui/chat/ChatScreenTest.kt | 9 ++ .../chat/fullbleed/FullBleedChatAnchorTest.kt | 61 ++++++-- .../chat/fullbleed/FullBleedChatListTest.kt | 53 ++++--- .../chat/fullbleed/TranscriptTestActions.kt | 21 +++ .../ui/chat/BrowserReturnTracker.kt | 44 ++++++ .../ui/chat/ChatConnectorsViewModel.kt | 11 ++ .../m57/hermescontrol/ui/chat/ChatScreen.kt | 143 ++++++------------ .../hermescontrol/ui/chat/ChatViewModel.kt | 21 +++ .../ui/chat/fullbleed/FullBleedChatList.kt | 110 ++++++-------- .../ui/chat/fullbleed/TranscriptUiState.kt | 99 ++++++++++++ .../ui/chat/BrowserReturnTrackerTest.kt | 62 ++++++++ .../chat/fullbleed/TranscriptUiStateTest.kt | 88 +++++++++++ 12 files changed, 521 insertions(+), 201 deletions(-) create mode 100644 app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/TranscriptTestActions.kt create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/chat/BrowserReturnTracker.kt create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/TranscriptUiState.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/BrowserReturnTrackerTest.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/fullbleed/TranscriptUiStateTest.kt diff --git a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/ChatScreenTest.kt b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/ChatScreenTest.kt index 52dd188f..564742ac 100644 --- a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/ChatScreenTest.kt +++ b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/ChatScreenTest.kt @@ -11,6 +11,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.filters.MediumTest import com.m57.hermescontrol.data.model.Attachment import com.m57.hermescontrol.data.ws.ConnectionStatus +import com.m57.hermescontrol.ui.chat.fullbleed.TranscriptUiState import com.m57.hermescontrol.ui.common.ActionProgressController import io.mockk.every import io.mockk.mockk @@ -61,6 +62,10 @@ class ChatScreenTest { every { mockViewModel.uiState } returns MutableStateFlow(uiState).asStateFlow() every { mockViewModel.streamingState } returns MutableStateFlow(StreamingState()).asStateFlow() every { mockViewModel.timelineState } returns MutableStateFlow(ChatTimelineState()).asStateFlow() + every { mockViewModel.transcriptState } returns + MutableStateFlow( + TranscriptUiState.resolve(uiState, ChatTimelineState(), StreamingState(), null, null), + ).asStateFlow() every { mockViewModel.connectionOperationState } returns MutableStateFlow(ConnectionOperationUiState()).asStateFlow() // ActionProgressDialog collects the controller's StateFlow — a relaxed @@ -110,6 +115,10 @@ class ChatScreenTest { every { mockViewModel.uiState } returns MutableStateFlow(uiState).asStateFlow() every { mockViewModel.streamingState } returns MutableStateFlow(StreamingState()).asStateFlow() every { mockViewModel.timelineState } returns MutableStateFlow(ChatTimelineState()).asStateFlow() + every { mockViewModel.transcriptState } returns + MutableStateFlow( + TranscriptUiState.resolve(uiState, ChatTimelineState(), StreamingState(), null, null), + ).asStateFlow() every { mockViewModel.connectionOperationState } returns MutableStateFlow(ConnectionOperationUiState()).asStateFlow() every { mockViewModel.actionProgress } returns diff --git a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatAnchorTest.kt b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatAnchorTest.kt index 58d09b5c..5a940cee 100644 --- a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatAnchorTest.kt +++ b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatAnchorTest.kt @@ -12,7 +12,8 @@ import androidx.compose.ui.unit.dp import androidx.test.ext.junit.runners.AndroidJUnit4 import com.m57.hermescontrol.ui.chat.ChatMessage import com.m57.hermescontrol.ui.chat.ChatSearchState -import com.m57.hermescontrol.ui.chat.ChatViewModel +import com.m57.hermescontrol.ui.chat.ChatTimelineState +import com.m57.hermescontrol.ui.chat.ChatUiState import com.m57.hermescontrol.ui.chat.ClarifyUi import com.m57.hermescontrol.ui.chat.MessageRole import com.m57.hermescontrol.ui.chat.StreamingState @@ -21,7 +22,6 @@ import com.m57.hermescontrol.ui.chat.VaultCodePromptUi import com.m57.hermescontrol.ui.chat.VaultSaveLoginPromptUi import com.m57.hermescontrol.ui.chat.VaultUnlockPromptUi import com.m57.hermescontrol.ui.chat.components.rememberChatScrollController -import io.mockk.mockk import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.launch @@ -116,29 +116,58 @@ class FullBleedChatAnchorTest { val tailKeys = listOf("agent_status", "clarify_bubble", "vault_unlock_card", "vault_save_login_card", "vault_code_card") val listState = LazyListState(index, offset) - val viewModel = mockk(relaxed = true) lateinit var scope: CoroutineScope compose.setContent { scope = rememberCoroutineScope() val controller = rememberChatScrollController(listState, scope) Box(Modifier.size(width = 320.dp, height = if (tailKey == null) 400.dp else 24.dp)) { FullBleedChatList( - messages = messages.value, - streamingState = streaming.value, - isAgentTyping = tailKey != null, + transcript = + TranscriptUiState.resolve( + chat = + ChatUiState( + messages = messages.value, + isAgentTyping = tailKey != null, + typingEffectEnabled = false, + typingEffectDelayMs = 30, + isLoadingOlder = loading.value, + currentSessionId = session.value, + clarifyRequest = + if (tailKey != + null + ) { + ClarifyUi("Choose an option", listOf("Yes")) + } else { + null + }, + vaultUnlockPrompt = + if (tailKey != null) { + VaultUnlockPromptUi( + null, + null, + ) + } else { + null + }, + vaultSaveLoginPrompt = + if (tailKey != + null + ) { + VaultSaveLoginPromptUi(null, null) + } else { + null + }, + vaultCodePrompt = if (tailKey != null) VaultCodePromptUi(null, null) else null, + ), + timeline = ChatTimelineState(), + streaming = streaming.value, + savingAttachmentPath = null, + speakingMessageId = null, + ), + actions = testTranscriptActions(), searchState = ChatSearchState(), - typingEffectEnabled = false, - typingEffectDelayMs = 30, - isLoading = false, - isLoadingOlder = loading.value, listState = listState, scrollController = controller, - viewModel = viewModel, - pagingSessionId = session.value, - clarifyRequest = if (tailKey != null) ClarifyUi("Choose an option", listOf("Yes")) else null, - vaultUnlockPrompt = if (tailKey != null) VaultUnlockPromptUi(null, null) else null, - vaultSaveLoginPrompt = if (tailKey != null) VaultSaveLoginPromptUi(null, null) else null, - vaultCodePrompt = if (tailKey != null) VaultCodePromptUi(null, null) else null, ) } } diff --git a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatListTest.kt b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatListTest.kt index b55870f2..a40a1c10 100644 --- a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatListTest.kt +++ b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatListTest.kt @@ -17,12 +17,12 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.filters.MediumTest import com.m57.hermescontrol.ui.chat.ChatMessage import com.m57.hermescontrol.ui.chat.ChatSearchState -import com.m57.hermescontrol.ui.chat.ChatViewModel +import com.m57.hermescontrol.ui.chat.ChatTimelineState +import com.m57.hermescontrol.ui.chat.ChatUiState import com.m57.hermescontrol.ui.chat.MessageRole import com.m57.hermescontrol.ui.chat.StreamingState import com.m57.hermescontrol.ui.chat.ToolStatus import com.m57.hermescontrol.ui.chat.components.ChatScrollController -import io.mockk.mockk import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import org.junit.Rule @@ -76,34 +76,41 @@ class FullBleedChatListTest { composeTestRule.setContent { val listState = LazyListState() FullBleedChatList( - messages = messages, - streamingState = streamingState, - isAgentTyping = isAgentTyping, + transcript = + TranscriptUiState.resolve( + chat = + ChatUiState( + messages = messages, + isAgentTyping = isAgentTyping, + typingEffectEnabled = false, + typingEffectDelayMs = 30, + messageStatsEnabled = messageStatsEnabled, + showUserMessageTokens = showUserMessageTokens, + showAssistantMessageTokens = showAssistantMessageTokens, + showTokensPerSecond = showTokensPerSecond, + clarifyRequest = + if (clarify) { + com.m57.hermescontrol.ui.chat.ClarifyUi( + text = "pick", + options = listOf("a"), + ) + } else { + null + }, + ), + timeline = ChatTimelineState(), + streaming = streamingState, + savingAttachmentPath = null, + speakingMessageId = null, + ), + actions = testTranscriptActions(), searchState = ChatSearchState(), - typingEffectEnabled = false, - typingEffectDelayMs = 30, - messageStatsEnabled = messageStatsEnabled, - showUserMessageTokens = showUserMessageTokens, - showAssistantMessageTokens = showAssistantMessageTokens, - showTokensPerSecond = showTokensPerSecond, - isLoading = false, - isLoadingOlder = false, listState = listState, scrollController = ChatScrollController( listState = listState, scope = CoroutineScope(Dispatchers.Main.immediate), ), - viewModel = mockk(relaxed = true), - clarifyRequest = - if (clarify) { - com.m57.hermescontrol.ui.chat.ClarifyUi( - text = "pick", - options = listOf("a"), - ) - } else { - null - }, ) } } diff --git a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/TranscriptTestActions.kt b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/TranscriptTestActions.kt new file mode 100644 index 00000000..90f29583 --- /dev/null +++ b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/TranscriptTestActions.kt @@ -0,0 +1,21 @@ +package com.m57.hermescontrol.ui.chat.fullbleed + +/** No-op actions for transcript layout tests; callbacks are irrelevant to their assertions. */ +internal fun testTranscriptActions() = + TranscriptActions( + onLoadOlder = {}, + onOpenAttachment = {}, + onSaveAttachment = {}, + onImageClick = {}, + onRespondApproval = {}, + onRespondClarify = {}, + onRespondClarifyBatch = {}, + onDismissClarify = {}, + onRespondVaultUnlock = {}, + onDismissVaultUnlock = {}, + onRespondVaultSaveLogin = { _, _ -> }, + onDismissVaultSaveLogin = {}, + onRespondVaultCode = {}, + onDismissVaultCode = {}, + onToggleSpeak = {}, + ) diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/BrowserReturnTracker.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/BrowserReturnTracker.kt new file mode 100644 index 00000000..da12f70c --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/BrowserReturnTracker.kt @@ -0,0 +1,44 @@ +package com.m57.hermescontrol.ui.chat + +/** Tracks a launched browser across pause/resume and screen disposal (issue #1321). */ +internal class BrowserReturnTracker { + data class Returned( + val operationId: String?, + ) + + private var inFlight = false + private var departed = false + private var operationId: String? = null + + fun start(operationId: String? = null) { + this.operationId = operationId + inFlight = true + departed = false + } + + fun onPause() { + if (inFlight) departed = true + } + + /** Only an actual departure followed by resume counts as returning from the browser. */ + fun onResume(): Returned? { + if (!inFlight || !departed) return null + val returned = Returned(operationId) + cancel() + return returned + } + + /** Drop the tracking after a failed launch; the lifecycle guard handles the failed lease. */ + fun cancel() { + inFlight = false + departed = false + operationId = null + } + + /** Non-configuration disposal must release an outstanding departed activity lease. */ + fun abandon(): Boolean { + val outstanding = inFlight && departed + cancel() + return outstanding + } +} diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatConnectorsViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatConnectorsViewModel.kt index 6aff6dab..f3052293 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatConnectorsViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatConnectorsViewModel.kt @@ -28,6 +28,17 @@ class ChatConnectorsViewModel( ) val uiState: StateFlow = delegateInstance.uiState + private val browserReturnTracker = BrowserReturnTracker() + + fun browserLaunched() = browserReturnTracker.start() + + fun browserLaunchFailed() = browserReturnTracker.cancel() + + fun browserPaused() = browserReturnTracker.onPause() + + fun browserReturned(): Boolean = browserReturnTracker.onResume() != null + + fun abandonBrowser(): Boolean = browserReturnTracker.abandon() init { var lastContext: Triple? = null diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt index 919ec93e..6f5baa49 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt @@ -123,6 +123,7 @@ import com.m57.hermescontrol.ui.chat.components.rememberChatSpeech import com.m57.hermescontrol.ui.chat.components.shouldShowProgressChip import com.m57.hermescontrol.ui.chat.components.tailContentKey import com.m57.hermescontrol.ui.chat.fullbleed.FullBleedChatList +import com.m57.hermescontrol.ui.chat.fullbleed.TranscriptActions import com.m57.hermescontrol.ui.common.ActionProgressDialog import com.m57.hermescontrol.ui.common.AutoScrollingTitleText import com.m57.hermescontrol.ui.common.CredentialWarningBanner @@ -168,6 +169,7 @@ fun ChatScreen( val state by viewModel.uiState.collectAsStateWithLifecycle() val streamingState by viewModel.streamingState.collectAsStateWithLifecycle() val timelineState by viewModel.timelineState.collectAsStateWithLifecycle() + val transcriptState by viewModel.transcriptState.collectAsStateWithLifecycle() val credentialWarning by HermesWsClient.credentialWarning.collectAsStateWithLifecycle() val connectorsViewModel: ChatConnectorsViewModel = viewModel() val connectorsState by connectorsViewModel.uiState.collectAsStateWithLifecycle() @@ -181,42 +183,26 @@ fun ChatScreen( val displayedMessages = timelineState.historyMessages ?: state.messages val lifecycleOwner = LocalLifecycleOwner.current val context = LocalContext.current - var browserAuthInFlight by rememberSaveable { mutableStateOf(false) } - var browserAuthDeparted by rememberSaveable { mutableStateOf(false) } - var connectionBrowserOperationId by remember { mutableStateOf(null) } - var connectionBrowserDeparted by remember { mutableStateOf(false) } DisposableEffect(lifecycleOwner) { val observer = LifecycleEventObserver { _, event -> when (event) { Lifecycle.Event.ON_PAUSE, Lifecycle.Event.ON_STOP -> { - if (browserAuthInFlight) { - browserAuthDeparted = true - } - if (connectionBrowserOperationId != null) { - connectionBrowserDeparted = true - } + connectorsViewModel.browserPaused() + viewModel.connectionBrowserPaused() connectorsViewModel.onPause() } Lifecycle.Event.ON_RESUME -> { viewModel.refreshSettings() connectorsViewModel.onResume() - val legacyBrowserReturned = browserAuthInFlight && browserAuthDeparted - val operationId = connectionBrowserOperationId.takeIf { connectionBrowserDeparted } + val legacyBrowserReturned = connectorsViewModel.browserReturned() + val operationId = viewModel.connectionBrowserReturned() if (legacyBrowserReturned || operationId != null) { ExternalActivityLifecycleGuard.externalActivityReturned() } - if (legacyBrowserReturned) { - browserAuthInFlight = false - browserAuthDeparted = false - } - if (operationId != null) { - connectionBrowserOperationId = null - connectionBrowserDeparted = false - viewModel.wakeConnectionOperation(operationId) - } + operationId?.let(viewModel::wakeConnectionOperation) } else -> {} @@ -231,18 +217,11 @@ fun ChatScreen( .firstOrNull() val isChangingConfigs = activity?.isChangingConfigurations == true if (!isChangingConfigs) { - val externalActivityOutstanding = - (browserAuthInFlight && browserAuthDeparted) || - (connectionBrowserOperationId != null && connectionBrowserDeparted) - if (externalActivityOutstanding) { + val legacyOutstanding = connectorsViewModel.abandonBrowser() + val connectionOutstanding = viewModel.abandonConnectionBrowser() + if (legacyOutstanding || connectionOutstanding) { ExternalActivityLifecycleGuard.externalActivityReturned() } - if (browserAuthInFlight && browserAuthDeparted) { - browserAuthInFlight = false - browserAuthDeparted = false - } - connectionBrowserOperationId = null - connectionBrowserDeparted = false connectorsViewModel.onPause() connectorsViewModel.hide() } @@ -413,21 +392,17 @@ fun ChatScreen( addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) } launchExternalActivity { - browserAuthDeparted = false - browserAuthInFlight = true + connectorsViewModel.browserLaunched() context.startActivity(intent) } } catch (_: ActivityNotFoundException) { - browserAuthInFlight = false - browserAuthDeparted = false + connectorsViewModel.browserLaunchFailed() connectorsViewModel.launchError(browserLaunchError) } catch (_: SecurityException) { - browserAuthInFlight = false - browserAuthDeparted = false + connectorsViewModel.browserLaunchFailed() connectorsViewModel.launchError(browserLaunchError) } catch (_: Exception) { - browserAuthInFlight = false - browserAuthDeparted = false + connectorsViewModel.browserLaunchFailed() connectorsViewModel.launchError(browserLaunchError) } } @@ -759,61 +734,41 @@ fun ChatScreen( // Full-bleed chat renderer (issue #866) — the single chat // surface since the bubble renderer was removed. FullBleedChatList( - messages = displayedMessages, - streamingState = if (timelineState.isHistorical) StreamingState() else streamingState, - isAgentTyping = state.isAgentTyping && !timelineState.isHistorical, + transcript = + transcriptState.copy( + savingAttachmentPath = pendingSavePath ?: transcriptState.savingAttachmentPath, + speakingMessageId = speakingMessageId, + ), + actions = + TranscriptActions( + onLoadOlder = viewModel::loadOlderMessages, + onOpenAttachment = viewModel::openAttachment, + onSaveAttachment = onSaveAttachment, + onImageClick = { viewingImage = it }, + onRespondApproval = viewModel::respondToApproval, + onRespondClarify = viewModel::respondToClarify, + onRespondClarifyBatch = viewModel::respondToClarifyBatch, + onDismissClarify = viewModel::dismissClarify, + onRespondVaultUnlock = viewModel::respondToVaultUnlock, + onDismissVaultUnlock = viewModel::dismissVaultUnlock, + onRespondVaultSaveLogin = viewModel::respondToVaultSaveLogin, + onDismissVaultSaveLogin = viewModel::dismissVaultSaveLogin, + onRespondVaultCode = viewModel::respondToVaultCode, + onDismissVaultCode = viewModel::dismissVaultCode, + onToggleSpeak = { message -> + val scopeKey = "${dataScope?.inMemoryKey(localKey = activeSessionId ?: "none")}" + speechController.toggle( + SpeechRequest( + scopeKey = scopeKey, + messageId = message.id, + text = SpeechText.stripMarkdownForSpeech(message.content), + ), + ) + }, + ), searchState = searchState, - typingEffectEnabled = state.typingEffectEnabled && !timelineState.isHistorical, - typingEffectDelayMs = state.typingEffectDelayMs, - messageStatsEnabled = state.messageStatsEnabled, - showUserMessageTokens = state.showUserMessageTokens, - showAssistantMessageTokens = state.showAssistantMessageTokens, - showTokensPerSecond = state.showTokensPerSecond, - maxToolCallsPerTurn = state.maxToolCallsPerTurn, - isLoading = state.isLoading && !timelineState.isHistorical, - isLoadingOlder = state.isLoadingOlder && !timelineState.isHistorical, - hasOlderMessages = state.hasOlderMessages && !timelineState.isHistorical, - pagingSessionId = - state.currentSessionId?.let { currentSessionId -> - if (timelineState.isHistorical) { - currentSessionId + ":history:" + timelineState.historyAnchorRowId - } else { - currentSessionId - } - }, listState = listState, scrollController = scrollController, - viewModel = viewModel, - clarifyRequest = state.clarifyRequest.takeUnless { timelineState.isHistorical }, - onRespondClarify = viewModel::respondToClarify, - onRespondClarifyBatch = viewModel::respondToClarifyBatch, - onDismissClarify = viewModel::dismissClarify, - vaultUnlockPrompt = state.vaultUnlockPrompt.takeUnless { timelineState.isHistorical }, - onRespondVaultUnlock = viewModel::respondToVaultUnlock, - onDismissVaultUnlock = viewModel::dismissVaultUnlock, - vaultSaveLoginPrompt = state.vaultSaveLoginPrompt.takeUnless { timelineState.isHistorical }, - onRespondVaultSaveLogin = viewModel::respondToVaultSaveLogin, - onDismissVaultSaveLogin = viewModel::dismissVaultSaveLogin, - vaultCodePrompt = state.vaultCodePrompt.takeUnless { timelineState.isHistorical }, - onRespondVaultCode = viewModel::respondToVaultCode, - onDismissVaultCode = viewModel::dismissVaultCode, - onSaveAttachment = onSaveAttachment, - savingAttachmentPath = pendingSavePath ?: state.savingAttachmentPath, - openingAttachmentPath = state.openingAttachmentPath, - isCompressing = state.isCompressing && !timelineState.isHistorical, - compressionStatus = state.compressionStatus.takeUnless { timelineState.isHistorical }, - onImageClick = { viewingImage = it }, - speakingMessageId = speakingMessageId, - onToggleSpeak = { message -> - val scopeKey = "${dataScope?.inMemoryKey(localKey = activeSessionId ?: "none")}" - speechController.toggle( - SpeechRequest( - scopeKey = scopeKey, - messageId = message.id, - text = SpeechText.stripMarkdownForSpeech(message.content), - ), - ) - }, replyErrorContent = state.replyFailure?.takeUnless { timelineState.isHistorical }?.let { failure -> { @@ -1084,8 +1039,7 @@ fun ChatScreen( if (ConnectorUrlValidator.isValidHttpsUrl(url)) { try { launchExternalActivity { - connectionBrowserOperationId = operationId - connectionBrowserDeparted = false + viewModel.connectionBrowserLaunched(operationId) val intent = Intent(Intent.ACTION_VIEW, Uri.parse(url)).apply { addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) @@ -1093,8 +1047,7 @@ fun ChatScreen( context.startActivity(intent) } } catch (_: Exception) { - connectionBrowserOperationId = null - connectionBrowserDeparted = false + viewModel.connectionBrowserLaunchFailed() scrollScope.launch { snackbarHostState.showSnackbar(browserLaunchError) } } } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt index 8a0d9c8e..69b4a1c4 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt @@ -40,6 +40,7 @@ import com.m57.hermescontrol.data.ws.toAny import com.m57.hermescontrol.data.ws.toJsonElement import com.m57.hermescontrol.notification.captureTurnBoundary import com.m57.hermescontrol.notification.correlationScopeId +import com.m57.hermescontrol.ui.chat.fullbleed.TranscriptUiState import com.m57.hermescontrol.ui.chat.tool.ToolViewCache import com.m57.hermescontrol.ui.common.ActionProgressController import kotlinx.coroutines.CancellationException @@ -452,6 +453,17 @@ class ChatViewModel( // ── Internal state ─────────────────────────────────────────────────── private val _uiState = MutableStateFlow(ChatUiState()) val connectionOperationState: StateFlow = connectionOperationDelegate.state + private val connectionBrowserReturnTracker = BrowserReturnTracker() + + fun connectionBrowserLaunched(operationId: String) = connectionBrowserReturnTracker.start(operationId) + + fun connectionBrowserLaunchFailed() = connectionBrowserReturnTracker.cancel() + + fun connectionBrowserPaused() = connectionBrowserReturnTracker.onPause() + + fun connectionBrowserReturned(): String? = connectionBrowserReturnTracker.onResume()?.operationId + + fun abandonConnectionBrowser(): Boolean = connectionBrowserReturnTracker.abandon() private val _streamingState = MutableStateFlow(StreamingState()) @@ -738,6 +750,15 @@ class ChatViewModel( _uiState.value, ) + val transcriptState: StateFlow = + combine(uiState, timelineState, streamingState) { chat, timeline, streaming -> + TranscriptUiState.resolve(chat, timeline, streaming, savingAttachmentPath = null, speakingMessageId = null) + }.stateIn( + viewModelScope, + SharingStarted.Eagerly, + TranscriptUiState.resolve(_uiState.value, _timelineState.value, _streamingState.value, null, null), + ) + init { // Parse tool payloads off the main thread before the transcript // composes them (issue #1327); ToolBubble then reads a cache hit. diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatList.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatList.kt index 1db9f185..998facc2 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatList.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatList.kt @@ -34,15 +34,9 @@ import com.m57.hermescontrol.R import com.m57.hermescontrol.theme.LocalChatFontScale import com.m57.hermescontrol.ui.chat.ChatMessage import com.m57.hermescontrol.ui.chat.ChatSearchState -import com.m57.hermescontrol.ui.chat.ChatViewModel -import com.m57.hermescontrol.ui.chat.ClarifyUi import com.m57.hermescontrol.ui.chat.ImageViewerModel -import com.m57.hermescontrol.ui.chat.StreamingState import com.m57.hermescontrol.ui.chat.ToolCallDivider import com.m57.hermescontrol.ui.chat.UserBubble -import com.m57.hermescontrol.ui.chat.VaultCodePromptUi -import com.m57.hermescontrol.ui.chat.VaultSaveLoginPromptUi -import com.m57.hermescontrol.ui.chat.VaultUnlockPromptUi import com.m57.hermescontrol.ui.chat.components.ChatHistoryPrefetch import com.m57.hermescontrol.ui.chat.components.ChatScrollController import com.m57.hermescontrol.ui.chat.components.ClarifyBubble @@ -78,48 +72,36 @@ private object FullBleedContentType { */ @Composable fun FullBleedChatList( - messages: List, - streamingState: StreamingState, - isAgentTyping: Boolean, + transcript: TranscriptUiState, + actions: TranscriptActions, searchState: ChatSearchState, - typingEffectEnabled: Boolean, - typingEffectDelayMs: Int, - messageStatsEnabled: Boolean = false, - showUserMessageTokens: Boolean = true, - showAssistantMessageTokens: Boolean = true, - showTokensPerSecond: Boolean = true, - maxToolCallsPerTurn: Int? = null, - isLoading: Boolean, - isLoadingOlder: Boolean, listState: androidx.compose.foundation.lazy.LazyListState, scrollController: ChatScrollController, - viewModel: ChatViewModel, - clarifyRequest: ClarifyUi? = null, - onRespondClarify: ((String) -> Unit)? = null, - onRespondClarifyBatch: ((Map) -> Unit)? = null, - onDismissClarify: (() -> Unit)? = null, - vaultUnlockPrompt: VaultUnlockPromptUi? = null, - onRespondVaultUnlock: ((String) -> Unit)? = null, - onDismissVaultUnlock: (() -> Unit)? = null, - vaultSaveLoginPrompt: VaultSaveLoginPromptUi? = null, - onRespondVaultSaveLogin: ((String, String) -> Unit)? = null, - onDismissVaultSaveLogin: (() -> Unit)? = null, - vaultCodePrompt: VaultCodePromptUi? = null, - onRespondVaultCode: ((String) -> Unit)? = null, - onDismissVaultCode: (() -> Unit)? = null, - onSaveAttachment: (com.m57.hermescontrol.data.model.Attachment) -> Unit = {}, - savingAttachmentPath: String? = null, - openingAttachmentPath: String? = null, - onImageClick: (ImageViewerModel) -> Unit = {}, - hasOlderMessages: Boolean = false, - pagingSessionId: String? = null, - onLoadOlder: () -> Unit = viewModel::loadOlderMessages, replyErrorContent: (@Composable () -> Unit)? = null, - speakingMessageId: String? = null, - onToggleSpeak: ((ChatMessage) -> Unit)? = null, - isCompressing: Boolean = false, - compressionStatus: String? = null, ) { + val messages = transcript.messages + val streamingState = transcript.streamingState + val isAgentTyping = transcript.isAgentTyping + val typingEffectEnabled = transcript.typingEffectEnabled + val typingEffectDelayMs = transcript.typingEffectDelayMs + val messageStatsEnabled = transcript.messageStatsEnabled + val showUserMessageTokens = transcript.showUserMessageTokens + val showAssistantMessageTokens = transcript.showAssistantMessageTokens + val showTokensPerSecond = transcript.showTokensPerSecond + val maxToolCallsPerTurn = transcript.maxToolCallsPerTurn + val isLoading = transcript.isLoading + val isLoadingOlder = transcript.isLoadingOlder + val hasOlderMessages = transcript.hasOlderMessages + val pagingSessionId = transcript.pagingSessionId + val clarifyRequest = transcript.clarifyRequest + val vaultUnlockPrompt = transcript.vaultUnlockPrompt + val vaultSaveLoginPrompt = transcript.vaultSaveLoginPrompt + val vaultCodePrompt = transcript.vaultCodePrompt + val savingAttachmentPath = transcript.savingAttachmentPath + val openingAttachmentPath = transcript.openingAttachmentPath + val isCompressing = transcript.isCompressing + val compressionStatus = transcript.compressionStatus + val speakingMessageId = transcript.speakingMessageId if (messages.isEmpty() && !isLoading && !isAgentTyping && replyErrorContent == null) { Box( modifier = Modifier.fillMaxSize(), @@ -189,9 +171,9 @@ fun FullBleedChatList( put("clarify_bubble") { ClarifyBubble( clarifyRequest = clarifyRequest, - onRespondSingle = { option -> onRespondClarify?.invoke(option) }, - onRespondBatch = { answers -> onRespondClarifyBatch?.invoke(answers) }, - onDismiss = { onDismissClarify?.invoke() }, + onRespondSingle = actions.onRespondClarify, + onRespondBatch = actions.onRespondClarifyBatch, + onDismiss = actions.onDismissClarify, ) } } @@ -200,8 +182,8 @@ fun FullBleedChatList( put("vault_unlock_card") { VaultUnlockCard( prompt = vaultUnlockPrompt, - onConfirm = { password -> onRespondVaultUnlock?.invoke(password) }, - onDismiss = { onDismissVaultUnlock?.invoke() }, + onConfirm = actions.onRespondVaultUnlock, + onDismiss = actions.onDismissVaultUnlock, ) } } @@ -210,10 +192,8 @@ fun FullBleedChatList( put("vault_save_login_card") { VaultSaveLoginCard( prompt = vaultSaveLoginPrompt, - onConfirm = { identifier, password -> - onRespondVaultSaveLogin?.invoke(identifier, password) - }, - onDismiss = { onDismissVaultSaveLogin?.invoke() }, + onConfirm = actions.onRespondVaultSaveLogin, + onDismiss = actions.onDismissVaultSaveLogin, ) } } @@ -222,8 +202,8 @@ fun FullBleedChatList( put("vault_code_card") { VaultCodeCard( prompt = vaultCodePrompt, - onConfirm = { code -> onRespondVaultCode?.invoke(code) }, - onDismiss = { onDismissVaultCode?.invoke() }, + onConfirm = actions.onRespondVaultCode, + onDismiss = actions.onDismissVaultCode, ) } } @@ -249,7 +229,7 @@ fun FullBleedChatList( } val prefetch = remember(pagingSessionId) { ChatHistoryPrefetch() } val canLoad = rememberUpdatedState(hasOlderMessages && !isLoadingOlder && !isLoading) - val loadOlder = rememberUpdatedState(onLoadOlder) + val loadOlder = rememberUpdatedState(actions.onLoadOlder) val prefetchConnection = remember(listState, prefetch, scrollController) { object : NestedScrollConnection { @@ -338,11 +318,11 @@ fun FullBleedChatList( isCurrentMatch = searchState.currentMatchId != null && searchState.currentMatchId == userMessage.id, - onOpenAttachment = viewModel::openAttachment, - onSaveAttachment = onSaveAttachment, + onOpenAttachment = actions.onOpenAttachment, + onSaveAttachment = actions.onSaveAttachment, savingAttachmentPath = savingAttachmentPath, openingAttachmentPath = openingAttachmentPath, - onImageClick = onImageClick, + onImageClick = actions.onImageClick, messageStatsEnabled = messageStatsEnabled, showUserMessageTokens = showUserMessageTokens, ) @@ -410,11 +390,7 @@ fun FullBleedChatList( speakingMessageId != null && speakingMessageId == proseMessage.id val toggleSpeakAction = - if (onToggleSpeak != null) { - { onToggleSpeak(proseMessage) } - } else { - null - } + { actions.onToggleSpeak(proseMessage) } FullBleedAgentMessage( message = proseMessage, // Highlight only bubbles that actually contain a match — @@ -431,12 +407,12 @@ fun FullBleedChatList( searchState.currentMatchId != null && searchState.currentMatchId == proseMessage.id, showReasoning = !hoistedReasoning, - onOpenAttachment = viewModel::openAttachment, - onSaveAttachment = onSaveAttachment, + onOpenAttachment = actions.onOpenAttachment, + onSaveAttachment = actions.onSaveAttachment, savingAttachmentPath = savingAttachmentPath, openingAttachmentPath = openingAttachmentPath, canSaveAttachment = savingAttachmentPath == null, - onImageClick = onImageClick, + onImageClick = actions.onImageClick, isSpeaking = isSpeakingThis, onToggleSpeak = toggleSpeakAction, messageStatsEnabled = messageStatsEnabled, @@ -480,7 +456,7 @@ fun FullBleedChatList( } else { FullBleedSystemEvent( message = sysMessage, - onRespondApproval = viewModel::respondToApproval, + onRespondApproval = actions.onRespondApproval, ) } } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/TranscriptUiState.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/TranscriptUiState.kt new file mode 100644 index 00000000..f02686fa --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/TranscriptUiState.kt @@ -0,0 +1,99 @@ +package com.m57.hermescontrol.ui.chat.fullbleed + +import com.m57.hermescontrol.data.model.Attachment +import com.m57.hermescontrol.ui.chat.ChatMessage +import com.m57.hermescontrol.ui.chat.ChatTimelineState +import com.m57.hermescontrol.ui.chat.ChatUiState +import com.m57.hermescontrol.ui.chat.ClarifyUi +import com.m57.hermescontrol.ui.chat.ImageViewerModel +import com.m57.hermescontrol.ui.chat.StreamingState +import com.m57.hermescontrol.ui.chat.VaultCodePromptUi +import com.m57.hermescontrol.ui.chat.VaultSaveLoginPromptUi +import com.m57.hermescontrol.ui.chat.VaultUnlockPromptUi + +/** The transcript's resolved, read-only state; the list never receives the whole chat ViewModel. */ +data class TranscriptUiState( + val messages: List, + val streamingState: StreamingState, + val isAgentTyping: Boolean, + val typingEffectEnabled: Boolean, + val typingEffectDelayMs: Int, + val messageStatsEnabled: Boolean, + val showUserMessageTokens: Boolean, + val showAssistantMessageTokens: Boolean, + val showTokensPerSecond: Boolean, + val maxToolCallsPerTurn: Int?, + val isLoading: Boolean, + val isLoadingOlder: Boolean, + val hasOlderMessages: Boolean, + val pagingSessionId: String?, + val clarifyRequest: ClarifyUi?, + val vaultUnlockPrompt: VaultUnlockPromptUi?, + val vaultSaveLoginPrompt: VaultSaveLoginPromptUi?, + val vaultCodePrompt: VaultCodePromptUi?, + val savingAttachmentPath: String?, + val openingAttachmentPath: String?, + val isCompressing: Boolean, + val compressionStatus: String?, + val speakingMessageId: String?, +) { + companion object { + /** Resolve historical versus live mode at the state boundary, not in the screen call. */ + fun resolve( + chat: ChatUiState, + timeline: ChatTimelineState, + streaming: StreamingState, + savingAttachmentPath: String?, + speakingMessageId: String?, + ): TranscriptUiState { + val historical = timeline.isHistorical + return TranscriptUiState( + messages = timeline.historyMessages ?: chat.messages, + streamingState = if (historical) StreamingState() else streaming, + isAgentTyping = !historical && chat.isAgentTyping, + typingEffectEnabled = !historical && chat.typingEffectEnabled, + typingEffectDelayMs = chat.typingEffectDelayMs, + messageStatsEnabled = chat.messageStatsEnabled, + showUserMessageTokens = chat.showUserMessageTokens, + showAssistantMessageTokens = chat.showAssistantMessageTokens, + showTokensPerSecond = chat.showTokensPerSecond, + maxToolCallsPerTurn = chat.maxToolCallsPerTurn, + isLoading = !historical && chat.isLoading, + isLoadingOlder = !historical && chat.isLoadingOlder, + hasOlderMessages = !historical && chat.hasOlderMessages, + pagingSessionId = + chat.currentSessionId?.let { id -> + if (historical) "$id:history:${timeline.historyAnchorRowId}" else id + }, + clarifyRequest = chat.clarifyRequest.takeUnless { historical }, + vaultUnlockPrompt = chat.vaultUnlockPrompt.takeUnless { historical }, + vaultSaveLoginPrompt = chat.vaultSaveLoginPrompt.takeUnless { historical }, + vaultCodePrompt = chat.vaultCodePrompt.takeUnless { historical }, + savingAttachmentPath = savingAttachmentPath ?: chat.savingAttachmentPath, + openingAttachmentPath = chat.openingAttachmentPath, + isCompressing = !historical && chat.isCompressing, + compressionStatus = chat.compressionStatus.takeUnless { historical }, + speakingMessageId = speakingMessageId, + ) + } + } +} + +/** All effects the transcript can request. The UI never reaches through to [ChatUiState]'s owner. */ +class TranscriptActions( + val onLoadOlder: () -> Unit, + val onOpenAttachment: (Attachment) -> Unit, + val onSaveAttachment: (Attachment) -> Unit, + val onImageClick: (ImageViewerModel) -> Unit, + val onRespondApproval: (String) -> Unit, + val onRespondClarify: (String) -> Unit, + val onRespondClarifyBatch: (Map) -> Unit, + val onDismissClarify: () -> Unit, + val onRespondVaultUnlock: (String) -> Unit, + val onDismissVaultUnlock: () -> Unit, + val onRespondVaultSaveLogin: (String, String) -> Unit, + val onDismissVaultSaveLogin: () -> Unit, + val onRespondVaultCode: (String) -> Unit, + val onDismissVaultCode: () -> Unit, + val onToggleSpeak: (ChatMessage) -> Unit, +) diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/BrowserReturnTrackerTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/BrowserReturnTrackerTest.kt new file mode 100644 index 00000000..31f1d62c --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/BrowserReturnTrackerTest.kt @@ -0,0 +1,62 @@ +package com.m57.hermescontrol.ui.chat + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +class BrowserReturnTrackerTest { + @Test + fun `browser return requires a departure and consumes the lease once`() { + val tracker = BrowserReturnTracker() + tracker.start("operation-1") + assertNull(tracker.onResume()) + tracker.onPause() + assertEquals("operation-1", tracker.onResume()?.operationId) + assertNull(tracker.onResume()) + assertFalse(tracker.abandon()) + } + + @Test + fun `legacy browser without operation id still reports return`() { + val tracker = BrowserReturnTracker() + tracker.start() + tracker.onPause() + assertTrue(tracker.onResume() != null) + assertNull(tracker.onResume()) + } + + @Test + fun `failed launch clears tracking and does not release a departed lease`() { + val tracker = BrowserReturnTracker() + tracker.start("operation-1") + tracker.onPause() + tracker.cancel() + assertNull(tracker.onResume()) + assertFalse(tracker.abandon()) + } + + @Test + fun `non configuration disposal releases only a departed lease and clears the id`() { + val tracker = BrowserReturnTracker() + tracker.start("operation-1") + assertFalse(tracker.abandon()) + tracker.start("operation-2") + tracker.onPause() + assertTrue(tracker.abandon()) + assertFalse(tracker.abandon()) + assertNull(tracker.onResume()) + } + + @Test + fun `new launch resets departed state from the previous launch`() { + val tracker = BrowserReturnTracker() + tracker.start("first") + tracker.onPause() + tracker.start("second") + assertNull(tracker.onResume()) + tracker.onPause() + assertEquals("second", tracker.onResume()?.operationId) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/fullbleed/TranscriptUiStateTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/fullbleed/TranscriptUiStateTest.kt new file mode 100644 index 00000000..5aef459d --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/fullbleed/TranscriptUiStateTest.kt @@ -0,0 +1,88 @@ +package com.m57.hermescontrol.ui.chat.fullbleed + +import com.m57.hermescontrol.ui.chat.ChatMessage +import com.m57.hermescontrol.ui.chat.ChatTimelineState +import com.m57.hermescontrol.ui.chat.ChatUiState +import com.m57.hermescontrol.ui.chat.ClarifyUi +import com.m57.hermescontrol.ui.chat.MessageRole +import com.m57.hermescontrol.ui.chat.StreamingState +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Test + +class TranscriptUiStateTest { + private val live = ChatMessage(id = "live", role = MessageRole.USER, content = "hello") + private val historical = ChatMessage(id = "historical", role = MessageRole.USER, content = "older") + private val clarify = ClarifyUi(text = "choose") + + @Test + fun `live state preserves prompts streaming and paging actions`() { + val stream = StreamingState() + val result = + TranscriptUiState.resolve( + chat = + ChatUiState( + messages = listOf(live), + currentSessionId = "session", + isAgentTyping = true, + isLoading = true, + isLoadingOlder = true, + hasOlderMessages = true, + clarifyRequest = clarify, + isCompressing = true, + savingAttachmentPath = "server-path", + ), + timeline = ChatTimelineState(), + streaming = stream, + savingAttachmentPath = "local-path", + speakingMessageId = "live", + ) + assertEquals(listOf(live), result.messages) + assertSame(stream, result.streamingState) + assertEquals("session", result.pagingSessionId) + assertTrue(result.isAgentTyping) + assertTrue(result.isLoading) + assertTrue(result.isLoadingOlder) + assertTrue(result.hasOlderMessages) + assertSame(clarify, result.clarifyRequest) + assertTrue(result.isCompressing) + assertEquals("local-path", result.savingAttachmentPath) + assertEquals("live", result.speakingMessageId) + } + + @Test + fun `historical state isolates live tail prompts loading and compression`() { + val result = + TranscriptUiState.resolve( + chat = + ChatUiState( + messages = listOf(live), + currentSessionId = "session", + isAgentTyping = true, + isLoading = true, + isLoadingOlder = true, + hasOlderMessages = true, + clarifyRequest = clarify, + isCompressing = true, + compressionStatus = "compressing", + ), + timeline = ChatTimelineState(historyMessages = listOf(historical), historyAnchorRowId = 42), + streaming = StreamingState(streamingMessage = live), + savingAttachmentPath = null, + speakingMessageId = null, + ) + assertEquals(listOf(historical), result.messages) + assertNull(result.streamingState.streamingMessage) + assertEquals("session:history:42", result.pagingSessionId) + assertFalse(result.isAgentTyping) + assertFalse(result.isLoading) + assertFalse(result.isLoadingOlder) + assertFalse(result.hasOlderMessages) + assertNull(result.clarifyRequest) + assertFalse(result.isCompressing) + assertNull(result.compressionStatus) + } +}