From 6d2362ba8aa4929191a35231056c61adc0f168ab Mon Sep 17 00:00:00 2001 From: M57 <41645758+Hy4ri@users.noreply.github.com> Date: Thu, 3 Sep 2026 21:38:57 +0300 Subject: [PATCH] feat(chat): support multi-select, batch multi-question clarify prompts, and expiry (fixes #1045) (#1055) --- .../m57/hermescontrol/data/ws/EventParser.kt | 101 ++++++-- .../com/m57/hermescontrol/data/ws/WsEvent.kt | 14 ++ .../m57/hermescontrol/ui/chat/ChatScreen.kt | 1 + .../hermescontrol/ui/chat/ChatViewModel.kt | 194 ++++++++++++--- .../ui/chat/ChatWsEventReducer.kt | 41 +++- .../ui/chat/components/MessageCards.kt | 232 ++++++++++++++---- .../ui/chat/fullbleed/FullBleedChatList.kt | 7 +- .../hermescontrol/data/ws/EventParserTest.kt | 73 ++++++ .../ui/chat/ChatViewModelTest.kt | 76 ++++++ .../ui/chat/ChatWsEventReducerTest.kt | 19 ++ 10 files changed, 649 insertions(+), 109 deletions(-) diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/EventParser.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/EventParser.kt index 55195363..3ae39754 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/EventParser.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/EventParser.kt @@ -136,31 +136,88 @@ object EventParser { // older client or test that still uses the legacy field names. (Issue #206) // Batch clarify (issue #18450): gateway sends "questions" array of {qid, question, choices, multi_select}. val rawQuestions = payload?.get("questions") as? List<*> - val firstQuestion = rawQuestions?.firstOrNull() as? Map<*, *> - - val text = - payload?.get("question") as? String - ?: payload?.get("text") as? String - ?: if (rawQuestions != null && rawQuestions.size > 1) { - rawQuestions - .mapIndexedNotNull { index, item -> - val q = (item as? Map<*, *>)?.get("question") as? String - q?.let { "${index + 1}. $it" } - }.joinToString("\n\n") - } else { - firstQuestion?.get("question") as? String - } - - val rawOptions = - payload?.get("choices") - ?: payload?.get("options") - ?: firstQuestion?.get("choices") val clarifyId = payload?.get("clarify_id") as? String ?: payload?.get("request_id") as? String - val questionId = firstQuestion?.get("qid") as? String + + val parsedQuestions = + if (rawQuestions != null && rawQuestions.isNotEmpty()) { + rawQuestions.mapIndexedNotNull { index, item -> + val map = item as? Map<*, *> ?: return@mapIndexedNotNull null + val qText = map["question"] as? String ?: return@mapIndexedNotNull null + val qid = map["qid"] as? String ?: "q$index" + + @Suppress("UNCHECKED_CAST") + val qChoices = (map["choices"] as? List<*>)?.filterIsInstance() ?: emptyList() + val qMulti = map["multi_select"] as? Boolean ?: false + WsEvent.ClarifyQuestion( + qid = qid, + question = qText, + choices = qChoices, + multiSelect = qMulti, + ) + } + } else { + val text = + payload?.get("question") as? String + ?: payload?.get("text") as? String + val rawOptions = payload?.get("choices") ?: payload?.get("options") + + @Suppress("UNCHECKED_CAST") + val options = (rawOptions as? List<*>)?.filterIsInstance() ?: emptyList() + val qid = payload?.get("qid") as? String ?: payload?.get("question_id") as? String + val multi = payload?.get("multi_select") as? Boolean ?: false + if ((!text.isNullOrBlank() || options.isNotEmpty()) && qid != null) { + listOf( + WsEvent.ClarifyQuestion( + qid = qid, + question = text.orEmpty(), + choices = options, + multiSelect = multi, + ), + ) + } else { + emptyList() + } + } + + val primary = parsedQuestions.firstOrNull() + val legacyText = + if (parsedQuestions.size > 1) { + parsedQuestions.mapIndexed { index, q -> "${index + 1}. ${q.question}" }.joinToString("\n\n") + } else if (primary != null) { + primary.question + } else { + payload?.get("question") as? String + ?: payload?.get("text") as? String + } @Suppress("UNCHECKED_CAST") - val options = (rawOptions as? List<*>)?.filterIsInstance() - WsEvent.ClarifyRequest(text, options, clarifyId, sessionId, questionId) + val legacyOptions = + primary?.choices + ?: (payload?.get("choices") ?: payload?.get("options")) + .let { (it as? List<*>)?.filterIsInstance() } + val legacyQid = + primary?.qid + ?: payload?.get("qid") as? String + ?: payload?.get("question_id") as? String + val legacyMulti = + primary?.multiSelect + ?: payload?.get("multi_select") as? Boolean + ?: false + + WsEvent.ClarifyRequest( + text = legacyText, + options = legacyOptions, + clarifyId = clarifyId, + sessionId = sessionId, + questionId = legacyQid, + multiSelect = legacyMulti, + questions = parsedQuestions, + ) + } + + "clarify.expire" -> { + val clarifyId = payload?.get("request_id") as? String ?: payload?.get("clarify_id") as? String + WsEvent.ClarifyExpire(clarifyId, sessionId) } "status.update" -> { diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/WsEvent.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/WsEvent.kt index 323040a3..5db47c98 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/WsEvent.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/WsEvent.kt @@ -135,12 +135,26 @@ sealed class WsEvent { // ── Interactive ────────────────────────────────────────────────────── + data class ClarifyQuestion( + val qid: String = "q0", + val question: String = "", + val choices: List = emptyList(), + val multiSelect: Boolean = false, + ) + data class ClarifyRequest( val text: String?, val options: List?, val clarifyId: String? = null, val sessionId: String? = null, val questionId: String? = null, + val multiSelect: Boolean = false, + val questions: List = emptyList(), + ) : WsEvent() + + data class ClarifyExpire( + val clarifyId: String? = null, + val sessionId: String? = null, ) : WsEvent() /** 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 01a83eaf..68d4f47f 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 @@ -720,6 +720,7 @@ fun ChatScreen( viewModel = viewModel, clarifyRequest = state.clarifyRequest, onRespondClarify = viewModel::respondToClarify, + onRespondClarifyBatch = viewModel::respondToClarifyBatch, onDismissClarify = viewModel::dismissClarify, onSaveAttachment = onSaveAttachment, savingAttachmentPath = pendingSavePath ?: state.savingAttachmentPath, 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 c64e3a16..a78a2825 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 @@ -377,12 +377,40 @@ data class SessionUi( val depth: Int = 0, ) +data class ClarifyQuestionUi( + val qid: String = "q0", + val question: String = "", + val choices: List = emptyList(), + val multiSelect: Boolean = false, +) + data class ClarifyUi( val text: String, - val options: List, + val options: List = emptyList(), val clarifyId: String? = null, val questionId: String? = null, -) + val multiSelect: Boolean = false, + val questions: List = emptyList(), +) { + /** + * Normalized list of questions to display. Guarantees at least one question + * entry even for legacy single-question clarify events. + */ + val resolvedQuestions: List + get() = + if (questions.isNotEmpty()) { + questions + } else { + listOf( + ClarifyQuestionUi( + qid = questionId ?: "q0", + question = text, + choices = options, + multiSelect = multiSelect, + ), + ) + } +} /** * State for the context-aware side-question bottom sheet (issue #1015, `/btw`). @@ -3377,44 +3405,107 @@ class ChatViewModel( */ fun dismissClarify() { val sessionId = _uiState.value.currentSessionId ?: return - val clarifyId = _uiState.value.clarifyRequest?.clarifyId - val questionId = _uiState.value.clarifyRequest?.questionId + val clarify = _uiState.value.clarifyRequest + val clarifyId = clarify?.clarifyId + // Raw batch questions (non-empty only for true batch payloads). + // Legacy singles keep questions empty and rely on questionId (nullable). + val isBatch = !clarify?.questions.isNullOrEmpty() + val displayQuestions = clarify?.resolvedQuestions.orEmpty() _uiState.update { it.copy(clarifyRequest = null) } addSystemMessage("Clarify dismissed — no answer sent", persist = true) viewModelScope.launch(ioDispatcher) { - val params = - mutableMapOf( - "session_id" to sessionId, - "response" to CLARIFY_DISMISS_RESPONSE, - "answer" to CLARIFY_DISMISS_RESPONSE, + if (isBatch) { + // Send dismissal for every question in the batch + for (q in displayQuestions) { + val params = + mutableMapOf( + "session_id" to sessionId, + "response" to CLARIFY_DISMISS_RESPONSE, + "answer" to CLARIFY_DISMISS_RESPONSE, + "question_id" to q.qid, + ) + if (clarifyId != null) { + params["clarify_id"] = clarifyId + params["request_id"] = clarifyId + } + wsClient.send( + method = WsMethods.CLARIFY_RESPOND, + params = params, + onSent = { id -> trackRequest(id, WsMethods.CLARIFY_RESPOND) }, + ) + } + } else { + // Legacy: only send question_id when the original payload had one. + val questionId = clarify?.questionId + val params = + mutableMapOf( + "session_id" to sessionId, + "response" to CLARIFY_DISMISS_RESPONSE, + "answer" to CLARIFY_DISMISS_RESPONSE, + ) + if (clarifyId != null) { + params["clarify_id"] = clarifyId + params["request_id"] = clarifyId + } + if (questionId != null) { + params["question_id"] = questionId + } + wsClient.send( + method = WsMethods.CLARIFY_RESPOND, + params = params, + onSent = { id -> trackRequest(id, WsMethods.CLARIFY_RESPOND) }, ) - if (clarifyId != null) { - params["clarify_id"] = clarifyId - params["request_id"] = clarifyId } - if (questionId != null) { - params["question_id"] = questionId - } - wsClient.send( - method = WsMethods.CLARIFY_RESPOND, - params = params, - onSent = { id -> trackRequest(id, WsMethods.CLARIFY_RESPOND) }, - ) } } fun respondToClarify(option: String) { + val clarify = _uiState.value.clarifyRequest + // Only use synthesized qid when this is a true batch or legacy with explicit qid. + val qid = + if (clarify != null && + (clarify.questionId != null || clarify.questions.isNotEmpty()) + ) { + clarify.resolvedQuestions.firstOrNull()?.qid ?: clarify.questionId + } else { + null + } + if (qid != null && clarify?.resolvedQuestions?.size == 1) { + respondToClarifyBatch(mapOf(qid to option)) + } else { + respondToClarifyBatch(emptyMap(), singleFallbackAnswer = option) + } + } + + fun respondToClarifyBatch( + answers: Map, + singleFallbackAnswer: String? = null, + ) { val sessionId = _uiState.value.currentSessionId ?: return - val clarifyId = _uiState.value.clarifyRequest?.clarifyId - val questionId = _uiState.value.clarifyRequest?.questionId + val clarify = _uiState.value.clarifyRequest + val clarifyId = clarify?.clarifyId + val isBatch = !clarify?.questions.isNullOrEmpty() + val questions = clarify?.resolvedQuestions.orEmpty() _uiState.update { it.copy(clarifyRequest = null) } + val displayContent = + if (questions.size > 1) { + questions + .mapIndexed { index, q -> + val ans = answers[q.qid]?.trim().orEmpty() + "${index + 1}. ${ans.ifEmpty { "(Skipped)" }}" + }.joinToString("\n") + } else { + val loneAns = answers.values.firstOrNull()?.trim() ?: singleFallbackAnswer?.trim().orEmpty() + loneAns + } + val userMessage = ChatMessage( role = MessageRole.USER, - content = option, + content = displayContent, ) _uiState.update { state -> @@ -3429,24 +3520,49 @@ class ChatViewModel( } viewModelScope.launch(ioDispatcher) { - val params = - mutableMapOf( - "session_id" to sessionId, - "response" to option, - "answer" to option, + if (isBatch) { + for (q in questions) { + val ans = answers[q.qid]?.trim().orEmpty() + val params = + mutableMapOf( + "session_id" to sessionId, + "response" to ans, + "answer" to ans, + "question_id" to q.qid, + ) + if (clarifyId != null) { + params["clarify_id"] = clarifyId + params["request_id"] = clarifyId + } + wsClient.send( + method = WsMethods.CLARIFY_RESPOND, + params = params, + onSent = { id -> trackRequest(id, WsMethods.CLARIFY_RESPOND) }, + ) + } + } else { + // Legacy single: only include question_id when explicitly present. + val qid = clarify?.questionId + val ans = answers.values.firstOrNull() ?: singleFallbackAnswer.orEmpty() + val params = + mutableMapOf( + "session_id" to sessionId, + "response" to ans, + "answer" to ans, + ) + if (clarifyId != null) { + params["clarify_id"] = clarifyId + params["request_id"] = clarifyId + } + if (qid != null) { + params["question_id"] = qid + } + wsClient.send( + method = WsMethods.CLARIFY_RESPOND, + params = params, + onSent = { id -> trackRequest(id, WsMethods.CLARIFY_RESPOND) }, ) - if (clarifyId != null) { - params["clarify_id"] = clarifyId - params["request_id"] = clarifyId } - if (questionId != null) { - params["question_id"] = questionId - } - wsClient.send( - method = WsMethods.CLARIFY_RESPOND, - params = params, - onSent = { id -> trackRequest(id, WsMethods.CLARIFY_RESPOND) }, - ) } } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducer.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducer.kt index 0ebf2a0c..50562f1e 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducer.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducer.kt @@ -67,6 +67,7 @@ object ChatWsEventReducer { is WsEvent.ToolComplete -> event.sessionId is WsEvent.ToolOutputRisk -> event.sessionId is WsEvent.ClarifyRequest -> event.sessionId + is WsEvent.ClarifyExpire -> event.sessionId is WsEvent.ToolProgress -> event.sessionId is WsEvent.ToolGenerating -> event.sessionId is WsEvent.SubagentEvent -> event.sessionId @@ -128,6 +129,8 @@ object ChatWsEventReducer { is WsEvent.ClarifyRequest -> onClarifyRequest(state, streamingState, event) + is WsEvent.ClarifyExpire -> onClarifyExpire(state, streamingState, event) + is WsEvent.ReviewSummary -> onReviewSummary(state, streamingState, event) is WsEvent.BtwComplete -> onBtwComplete(state, streamingState, event) @@ -636,8 +639,25 @@ object ChatWsEventReducer { state: ChatUiState, streamingState: StreamingState, event: WsEvent.ClarifyRequest, - ): ReducerResult = - ReducerResult( + ): ReducerResult { + val questions = + if (event.questions.isNotEmpty()) { + event.questions.map { + ClarifyQuestionUi( + qid = it.qid, + question = it.question, + choices = it.choices, + multiSelect = it.multiSelect, + ) + } + } else { + // Legacy single-question payload: keep questions empty so the wire + // layer can tell legacy apart from a true batch. UI synthesizes + // via ClarifyUi.resolvedQuestions; wire uses text/options/questionId. + emptyList() + } + + return ReducerResult( state = state.copy( clarifyRequest = @@ -646,10 +666,27 @@ object ChatWsEventReducer { options = event.options.orEmpty(), clarifyId = event.clarifyId, questionId = event.questionId, + multiSelect = event.multiSelect, + questions = questions, ), isAgentTyping = false, ), ) + } + + private fun onClarifyExpire( + state: ChatUiState, + streamingState: StreamingState, + event: WsEvent.ClarifyExpire, + ): ReducerResult = + if (event.clarifyId == null || state.clarifyRequest?.clarifyId == event.clarifyId) { + ReducerResult( + state = state.copy(clarifyRequest = null), + streamingState = streamingState, + ) + } else { + ReducerResult(state = state, streamingState = streamingState) + } // ── ReviewSummary (Self-improvement background review) ────────────── diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/MessageCards.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/MessageCards.kt index 473338b0..8ec0a493 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/MessageCards.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/MessageCards.kt @@ -42,6 +42,8 @@ import androidx.compose.material3.Card import androidx.compose.material3.CardDefaults import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.FilledTonalButton +import androidx.compose.material3.FilterChip +import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme @@ -85,6 +87,7 @@ import com.m57.hermescontrol.theme.CodeTerminalBg import com.m57.hermescontrol.theme.CodeTerminalBorder import com.m57.hermescontrol.theme.CodeTerminalMuted import com.m57.hermescontrol.theme.CodeTerminalText +import com.m57.hermescontrol.ui.chat.ClarifyUi import com.m57.hermescontrol.ui.chat.SubagentIndicator import kotlinx.coroutines.delay @@ -429,19 +432,27 @@ private fun copyToClipboard( // ── ClarifyBubble ───────────────────────────────────────────────────────── /** - * Centered dashed-border bubble showing a clarify question with - * selectable option chips and a dismiss button. + * Centered dashed-border bubble showing a clarify question (or batch of questions) with + * selectable option chips, multi-select toggles, open-ended input, and a dismiss button. */ @OptIn(ExperimentalLayoutApi::class) @Composable fun ClarifyBubble( - text: String, - options: List, - onOptionSelected: (String) -> Unit, + clarifyRequest: ClarifyUi, + onRespondSingle: (String) -> Unit, + onRespondBatch: (Map) -> Unit, onDismiss: () -> Unit, modifier: Modifier = Modifier, ) { - var typedText by remember { mutableStateOf("") } + val questions = clarifyRequest.resolvedQuestions + val isBatch = questions.size > 1 + + var selectedChoicesByQid by remember(clarifyRequest) { + mutableStateOf>>(emptyMap()) + } + var customTextByQid by remember(clarifyRequest) { + mutableStateOf>(emptyMap()) + } Surface( modifier = @@ -458,57 +469,170 @@ fun ClarifyBubble( ), ) { Column( - modifier = Modifier.padding(12.dp), + modifier = Modifier.padding(14.dp), horizontalAlignment = Alignment.CenterHorizontally, ) { - Text( - text = text, - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurface, - ) - if (options.isNotEmpty()) { - Spacer(Modifier.height(10.dp)) - FlowRow( - modifier = Modifier.fillMaxWidth(), - horizontalArrangement = Arrangement.spacedBy(6.dp), - verticalArrangement = Arrangement.spacedBy(6.dp), - ) { - options.forEach { option -> - SuggestionChip( - onClick = { onOptionSelected(option) }, - label = { Text(option) }, - colors = - SuggestionChipDefaults.suggestionChipColors( - containerColor = MaterialTheme.colorScheme.secondaryContainer, - labelColor = MaterialTheme.colorScheme.onSecondaryContainer, - ), - ) + questions.forEachIndexed { index, q -> + if (index > 0) { + HorizontalDivider( + modifier = Modifier.padding(vertical = 12.dp), + color = MaterialTheme.colorScheme.outlineVariant.copy(alpha = 0.5f), + ) + } + + if (isBatch) { + Text( + text = "${index + 1}. ${q.question}", + style = MaterialTheme.typography.bodyMedium, + fontWeight = FontWeight.SemiBold, + color = MaterialTheme.colorScheme.onSurface, + modifier = Modifier.fillMaxWidth(), + ) + } else { + Text( + text = q.question, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurface, + modifier = Modifier.fillMaxWidth(), + ) + } + + if (q.multiSelect) { + Spacer(Modifier.height(2.dp)) + Text( + text = "Select all that apply", + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.fillMaxWidth(), + ) + } + + if (q.choices.isNotEmpty()) { + Spacer(Modifier.height(8.dp)) + FlowRow( + modifier = Modifier.fillMaxWidth(), + horizontalArrangement = Arrangement.spacedBy(6.dp), + verticalArrangement = Arrangement.spacedBy(6.dp), + ) { + q.choices.forEach { choice -> + val selectedSet = selectedChoicesByQid[q.qid] ?: emptySet() + val isSelected = selectedSet.contains(choice) + FilterChip( + selected = isSelected, + onClick = { + if (q.multiSelect) { + val updated = if (isSelected) selectedSet - choice else selectedSet + choice + selectedChoicesByQid = selectedChoicesByQid + (q.qid to updated) + } else { + if (!isBatch && customTextByQid[q.qid].isNullOrBlank()) { + // Fast 1-tap respond for lone single-select question when no custom text is entered + onRespondSingle(choice) + } else { + val updated = if (isSelected) emptySet() else setOf(choice) + selectedChoicesByQid = selectedChoicesByQid + (q.qid to updated) + } + } + }, + label = { Text(choice) }, + leadingIcon = + if (isSelected) { + { + Icon( + imageVector = Icons.Default.Check, + contentDescription = null, + modifier = Modifier.size(16.dp), + ) + } + } else { + null + }, + ) + } } } + + Spacer(Modifier.height(8.dp)) + val typed = customTextByQid[q.qid].orEmpty() + OutlinedTextField( + value = typed, + onValueChange = { newText -> + customTextByQid = customTextByQid + (q.qid to newText) + }, + label = { + Text( + if (q.choices.isEmpty()) { + stringResource(R.string.message_your_response) + } else { + "Other (optional)" + }, + ) + }, + modifier = Modifier.fillMaxWidth(), + singleLine = true, + ) } - Spacer(Modifier.height(10.dp)) - OutlinedTextField( - value = typedText, - onValueChange = { typedText = it }, - label = { Text(stringResource(R.string.message_your_response)) }, - modifier = Modifier.fillMaxWidth(), - singleLine = true, - ) - Spacer(Modifier.height(8.dp)) + + Spacer(Modifier.height(12.dp)) Row( modifier = Modifier.fillMaxWidth(), horizontalArrangement = Arrangement.spacedBy(8.dp, Alignment.CenterHorizontally), ) { + val hasAnyInput = + questions.any { q -> + (selectedChoicesByQid[q.qid]?.isNotEmpty() == true) || + (!customTextByQid[q.qid].isNullOrBlank()) + } + FilledTonalButton( onClick = { - if (typedText.isNotBlank()) { - onOptionSelected(typedText) - typedText = "" + if (isBatch) { + val answers = + questions.associate { q -> + val selectedList = selectedChoicesByQid[q.qid]?.toList().orEmpty() + val custom = customTextByQid[q.qid]?.trim().orEmpty() + val finalAns = + when { + selectedList.isNotEmpty() && custom.isNotEmpty() -> { + (selectedList + custom).joinToString(", ") + } + + selectedList.isNotEmpty() -> { + selectedList.joinToString(", ") + } + + else -> { + custom + } + } + q.qid to finalAns + } + onRespondBatch(answers) + } else { + val q = questions.first() + val selectedList = selectedChoicesByQid[q.qid]?.toList().orEmpty() + val custom = customTextByQid[q.qid]?.trim().orEmpty() + val finalAns = + when { + selectedList.isNotEmpty() && custom.isNotEmpty() -> { + (selectedList + custom).joinToString(", ") + } + + selectedList.isNotEmpty() -> { + selectedList.joinToString(", ") + } + + else -> { + custom + } + } + if (finalAns.isNotBlank()) { + onRespondSingle(finalAns) + } } }, - enabled = typedText.isNotBlank(), + enabled = hasAnyInput, ) { - Text("Send") + Text(if (isBatch) "Submit All" else "Send") } TextButton(onClick = onDismiss) { Text("Dismiss") @@ -518,6 +642,28 @@ fun ClarifyBubble( } } +/** + * Backward-compatible overload for legacy callers and unit tests. + */ +@Composable +fun ClarifyBubble( + text: String, + options: List, + onOptionSelected: (String) -> Unit, + onDismiss: () -> Unit, + modifier: Modifier = Modifier, +) { + ClarifyBubble( + clarifyRequest = ClarifyUi(text = text, options = options), + onRespondSingle = onOptionSelected, + onRespondBatch = { answers -> + answers.values.firstOrNull()?.let(onOptionSelected) + }, + onDismiss = onDismiss, + modifier = modifier, + ) +} + // ── SubagentCard ────────────────────────────────────────────────────────── /** 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 5f3eccdf..5de6167d 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 @@ -67,6 +67,7 @@ fun FullBleedChatList( viewModel: ChatViewModel, clarifyRequest: ClarifyUi? = null, onRespondClarify: ((String) -> Unit)? = null, + onRespondClarifyBatch: ((Map) -> Unit)? = null, onDismissClarify: (() -> Unit)? = null, onSaveAttachment: (com.m57.hermescontrol.data.model.Attachment) -> Unit = {}, savingAttachmentPath: String? = null, @@ -299,9 +300,9 @@ fun FullBleedChatList( if (clarifyRequest != null) { item(key = "clarify_bubble") { ClarifyBubble( - text = clarifyRequest.text, - options = clarifyRequest.options, - onOptionSelected = { option -> onRespondClarify?.invoke(option) }, + clarifyRequest = clarifyRequest, + onRespondSingle = { option -> onRespondClarify?.invoke(option) }, + onRespondBatch = { answers -> onRespondClarifyBatch?.invoke(answers) }, onDismiss = { onDismissClarify?.invoke() }, ) } diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/EventParserTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/EventParserTest.kt index 675fdd8f..98d595e3 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/EventParserTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/EventParserTest.kt @@ -327,6 +327,79 @@ class EventParserTest { assertEquals("q0", clarifyEvent.questionId) } + @Test + fun testParseClarifyRequest_withMultiSelectAndMultipleQuestions() { + val response = + createJsonRpcResponse( + jsonrpc = "2.0", + id = null, + result = null, + error = null, + method = "event", + params = + mapOf( + "type" to "clarify.request", + "payload" to + mapOf( + "request_id" to "req-batch-2", + "questions" to + listOf( + mapOf( + "qid" to "q0", + "question" to "Pick languages:", + "choices" to listOf("Python", "Go", "Rust"), + "multi_select" to true, + ), + mapOf( + "qid" to "q1", + "question" to "Any comments?", + "choices" to emptyList(), + "multi_select" to false, + ), + ), + ), + ), + ) + val event = EventParser.parse(response) + assertTrue(event is WsEvent.ClarifyRequest) + val clarifyEvent = event as WsEvent.ClarifyRequest + assertEquals("req-batch-2", clarifyEvent.clarifyId) + assertEquals(2, clarifyEvent.questions.size) + + val q0 = clarifyEvent.questions[0] + assertEquals("q0", q0.qid) + assertEquals("Pick languages:", q0.question) + assertEquals(listOf("Python", "Go", "Rust"), q0.choices) + assertTrue(q0.multiSelect) + + val q1 = clarifyEvent.questions[1] + assertEquals("q1", q1.qid) + assertEquals("Any comments?", q1.question) + assertTrue(q1.choices.isEmpty()) + assertFalse(q1.multiSelect) + } + + @Test + fun testParseClarifyExpire_parsesSuccessfully() { + val response = + createJsonRpcResponse( + jsonrpc = "2.0", + id = null, + result = null, + error = null, + method = "event", + params = + mapOf( + "type" to "clarify.expire", + "payload" to mapOf("request_id" to "req-expire-1"), + ), + ) + val event = EventParser.parse(response) + assertTrue(event is WsEvent.ClarifyExpire) + val expireEvent = event as WsEvent.ClarifyExpire + assertEquals("req-expire-1", expireEvent.clarifyId) + } + // ── TEST-07: Untested subtypes ───────────────────────────────────── @Test diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt index 1c202185..8c453580 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt @@ -1407,6 +1407,82 @@ class ChatViewModelTest { } } + @Test + fun testClarifyBatch_respondsToAllQuestions() = + runTest { + val (viewModel, sessionId) = createViewModelWithSession() + + mockEventsFlow.emit( + WsEvent.ClarifyRequest( + text = "1. Pick A\n\n2. Pick B", + options = listOf("A1", "A2"), + clarifyId = "clarify-batch-multi", + sessionId = sessionId, + questionId = "q0", + questions = + listOf( + WsEvent.ClarifyQuestion( + qid = "q0", + question = "Pick A", + choices = listOf("A1", "A2"), + multiSelect = false, + ), + WsEvent.ClarifyQuestion( + qid = "q1", + question = "Pick B", + choices = listOf("B1", "B2"), + multiSelect = true, + ), + ), + ), + ) + advanceUntilIdle() + + val clarify = viewModel.uiState.value.clarifyRequest + assertNotNull(clarify) + assertEquals(2, clarify?.resolvedQuestions?.size) + + viewModel.respondToClarifyBatch( + mapOf( + "q0" to "A1", + "q1" to "B1, B2", + ), + ) + advanceUntilIdle() + + assertNull(viewModel.uiState.value.clarifyRequest) + verify { + HermesWsClient.send( + method = WsMethods.CLARIFY_RESPOND, + params = + mapOf( + "session_id" to sessionId, + "response" to "A1", + "answer" to "A1", + "question_id" to "q0", + "clarify_id" to "clarify-batch-multi", + "request_id" to "clarify-batch-multi", + ), + onSent = any(), + ) + } + verify { + HermesWsClient.send( + method = WsMethods.CLARIFY_RESPOND, + params = + mapOf( + "session_id" to sessionId, + "response" to "B1, B2", + "answer" to "B1, B2", + "question_id" to "q1", + "clarify_id" to "clarify-batch-multi", + "request_id" to "clarify-batch-multi", + ), + onSent = any(), + ) + } + } + @Test fun testClarifyRequestCustomResponse() = runTest { diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducerTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducerTest.kt index eec260cd..27baf301 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducerTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatWsEventReducerTest.kt @@ -26,6 +26,25 @@ class ChatWsEventReducerTest { assertEquals(null, result.state.clarifyRequest) } + @Test + fun testClarifyExpire_clearsMatchingClarifyRequest() { + val state = + ChatUiState( + currentSessionId = "session-1", + clarifyRequest = ClarifyUi("Expiring question", emptyList(), "clarify-expire-1"), + ) + + val result = + ChatWsEventReducer.reduce( + state = state, + streamingState = StreamingState(), + event = WsEvent.ClarifyExpire(clarifyId = "clarify-expire-1", sessionId = "session-1"), + currentSessionId = "session-1", + ) + + assertEquals(null, result.state.clarifyRequest) + } + @Test fun testToolProgress_updatesProgressPreviewForMatchingRunningTool() { val initialMessage =