fix(chat): support batch clarify.request payload and route question_id on respond (fixes #1045) (#1046)
This commit is contained in:
6 files changed
+159
-1
No files matched your search
@@ -134,17 +134,33 @@ object EventParser {
|
|||||||
"clarify.request" -> {
|
"clarify.request" -> {
|
||||||
// Gateway sends "question"/"choices" — fall back to "text"/"options" for any
|
// Gateway sends "question"/"choices" — fall back to "text"/"options" for any
|
||||||
// older client or test that still uses the legacy field names. (Issue #206)
|
// 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 =
|
val text =
|
||||||
payload?.get("question") as? String
|
payload?.get("question") as? String
|
||||||
?: payload?.get("text") 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 =
|
val rawOptions =
|
||||||
payload?.get("choices")
|
payload?.get("choices")
|
||||||
?: payload?.get("options")
|
?: payload?.get("options")
|
||||||
|
?: firstQuestion?.get("choices")
|
||||||
val clarifyId = payload?.get("clarify_id") as? String ?: payload?.get("request_id") as? String
|
val clarifyId = payload?.get("clarify_id") as? String ?: payload?.get("request_id") as? String
|
||||||
|
val questionId = firstQuestion?.get("qid") as? String
|
||||||
|
|
||||||
@Suppress("UNCHECKED_CAST")
|
@Suppress("UNCHECKED_CAST")
|
||||||
val options = (rawOptions as? List<*>)?.filterIsInstance<String>()
|
val options = (rawOptions as? List<*>)?.filterIsInstance<String>()
|
||||||
WsEvent.ClarifyRequest(text, options, clarifyId, sessionId)
|
WsEvent.ClarifyRequest(text, options, clarifyId, sessionId, questionId)
|
||||||
}
|
}
|
||||||
|
|
||||||
"status.update" -> {
|
"status.update" -> {
|
||||||
|
|||||||
@@ -140,6 +140,7 @@ sealed class WsEvent {
|
|||||||
val options: List<String>?,
|
val options: List<String>?,
|
||||||
val clarifyId: String? = null,
|
val clarifyId: String? = null,
|
||||||
val sessionId: String? = null,
|
val sessionId: String? = null,
|
||||||
|
val questionId: String? = null,
|
||||||
) : WsEvent()
|
) : WsEvent()
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -379,6 +379,7 @@ data class ClarifyUi(
|
|||||||
val text: String,
|
val text: String,
|
||||||
val options: List<String>,
|
val options: List<String>,
|
||||||
val clarifyId: String? = null,
|
val clarifyId: String? = null,
|
||||||
|
val questionId: String? = null,
|
||||||
)
|
)
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -3236,6 +3237,7 @@ class ChatViewModel(
|
|||||||
fun dismissClarify() {
|
fun dismissClarify() {
|
||||||
val sessionId = _uiState.value.currentSessionId ?: return
|
val sessionId = _uiState.value.currentSessionId ?: return
|
||||||
val clarifyId = _uiState.value.clarifyRequest?.clarifyId
|
val clarifyId = _uiState.value.clarifyRequest?.clarifyId
|
||||||
|
val questionId = _uiState.value.clarifyRequest?.questionId
|
||||||
_uiState.update { it.copy(clarifyRequest = null) }
|
_uiState.update { it.copy(clarifyRequest = null) }
|
||||||
|
|
||||||
addSystemMessage("Clarify dismissed — no answer sent", persist = true)
|
addSystemMessage("Clarify dismissed — no answer sent", persist = true)
|
||||||
@@ -3251,6 +3253,9 @@ class ChatViewModel(
|
|||||||
params["clarify_id"] = clarifyId
|
params["clarify_id"] = clarifyId
|
||||||
params["request_id"] = clarifyId
|
params["request_id"] = clarifyId
|
||||||
}
|
}
|
||||||
|
if (questionId != null) {
|
||||||
|
params["question_id"] = questionId
|
||||||
|
}
|
||||||
wsClient.send(
|
wsClient.send(
|
||||||
method = WsMethods.CLARIFY_RESPOND,
|
method = WsMethods.CLARIFY_RESPOND,
|
||||||
params = params,
|
params = params,
|
||||||
@@ -3262,6 +3267,7 @@ class ChatViewModel(
|
|||||||
fun respondToClarify(option: String) {
|
fun respondToClarify(option: String) {
|
||||||
val sessionId = _uiState.value.currentSessionId ?: return
|
val sessionId = _uiState.value.currentSessionId ?: return
|
||||||
val clarifyId = _uiState.value.clarifyRequest?.clarifyId
|
val clarifyId = _uiState.value.clarifyRequest?.clarifyId
|
||||||
|
val questionId = _uiState.value.clarifyRequest?.questionId
|
||||||
_uiState.update { it.copy(clarifyRequest = null) }
|
_uiState.update { it.copy(clarifyRequest = null) }
|
||||||
|
|
||||||
val userMessage =
|
val userMessage =
|
||||||
@@ -3292,6 +3298,9 @@ class ChatViewModel(
|
|||||||
params["clarify_id"] = clarifyId
|
params["clarify_id"] = clarifyId
|
||||||
params["request_id"] = clarifyId
|
params["request_id"] = clarifyId
|
||||||
}
|
}
|
||||||
|
if (questionId != null) {
|
||||||
|
params["question_id"] = questionId
|
||||||
|
}
|
||||||
wsClient.send(
|
wsClient.send(
|
||||||
method = WsMethods.CLARIFY_RESPOND,
|
method = WsMethods.CLARIFY_RESPOND,
|
||||||
params = params,
|
params = params,
|
||||||
|
|||||||
@@ -633,6 +633,7 @@ object ChatWsEventReducer {
|
|||||||
text = event.text.orEmpty(),
|
text = event.text.orEmpty(),
|
||||||
options = event.options.orEmpty(),
|
options = event.options.orEmpty(),
|
||||||
clarifyId = event.clarifyId,
|
clarifyId = event.clarifyId,
|
||||||
|
questionId = event.questionId,
|
||||||
),
|
),
|
||||||
isAgentTyping = false,
|
isAgentTyping = false,
|
||||||
),
|
),
|
||||||
|
|||||||
@@ -291,6 +291,42 @@ class EventParserTest {
|
|||||||
assertEquals(listOf("staging", "production"), clarifyEvent.options)
|
assertEquals(listOf("staging", "production"), clarifyEvent.options)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun testParseClarifyRequest_withBatchQuestions_parsesSuccessfully() {
|
||||||
|
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-1",
|
||||||
|
"questions" to
|
||||||
|
listOf(
|
||||||
|
mapOf(
|
||||||
|
"qid" to "q0",
|
||||||
|
"question" to "Which database?",
|
||||||
|
"choices" to listOf("Postgres", "SQLite"),
|
||||||
|
"multi_select" to false,
|
||||||
|
),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
val event = EventParser.parse(response)
|
||||||
|
assertTrue(event is WsEvent.ClarifyRequest)
|
||||||
|
val clarifyEvent = event as WsEvent.ClarifyRequest
|
||||||
|
assertEquals("Which database?", clarifyEvent.text)
|
||||||
|
assertEquals(listOf("Postgres", "SQLite"), clarifyEvent.options)
|
||||||
|
assertEquals("req-batch-1", clarifyEvent.clarifyId)
|
||||||
|
assertEquals("q0", clarifyEvent.questionId)
|
||||||
|
}
|
||||||
|
|
||||||
// ── TEST-07: Untested subtypes ─────────────────────────────────────
|
// ── TEST-07: Untested subtypes ─────────────────────────────────────
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
@@ -1217,6 +1217,54 @@ class ChatViewModelTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun testClarifyRequestWithQuestionIdAndRespond() =
|
||||||
|
runTest {
|
||||||
|
val (viewModel, sessionId) = createViewModelWithSession()
|
||||||
|
|
||||||
|
mockEventsFlow.emit(
|
||||||
|
WsEvent.ClarifyRequest(
|
||||||
|
text = "Pick one:",
|
||||||
|
options = listOf("Option 1"),
|
||||||
|
clarifyId = "clarify-batch-1",
|
||||||
|
sessionId = sessionId,
|
||||||
|
questionId = "q0",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
advanceUntilIdle()
|
||||||
|
|
||||||
|
assertEquals(
|
||||||
|
"Pick one:",
|
||||||
|
viewModel.uiState.value.clarifyRequest
|
||||||
|
?.text,
|
||||||
|
)
|
||||||
|
assertEquals(
|
||||||
|
"q0",
|
||||||
|
viewModel.uiState.value.clarifyRequest
|
||||||
|
?.questionId,
|
||||||
|
)
|
||||||
|
|
||||||
|
viewModel.respondToClarify("Option 1")
|
||||||
|
advanceUntilIdle()
|
||||||
|
|
||||||
|
assertNull(viewModel.uiState.value.clarifyRequest)
|
||||||
|
verify {
|
||||||
|
HermesWsClient.send(
|
||||||
|
method = WsMethods.CLARIFY_RESPOND,
|
||||||
|
params =
|
||||||
|
mapOf(
|
||||||
|
"session_id" to sessionId,
|
||||||
|
"response" to "Option 1",
|
||||||
|
"answer" to "Option 1",
|
||||||
|
"clarify_id" to "clarify-batch-1",
|
||||||
|
"request_id" to "clarify-batch-1",
|
||||||
|
"question_id" to "q0",
|
||||||
|
),
|
||||||
|
onSent = any(),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun testClarifyRequestCustomResponse() =
|
fun testClarifyRequestCustomResponse() =
|
||||||
runTest {
|
runTest {
|
||||||
@@ -1316,6 +1364,53 @@ class ChatViewModelTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun testClarifyDismissWithQuestionIdInformsAgent() =
|
||||||
|
runTest {
|
||||||
|
val (viewModel, sessionId) = createViewModelWithSession()
|
||||||
|
|
||||||
|
mockEventsFlow.emit(
|
||||||
|
WsEvent.ClarifyRequest(
|
||||||
|
"Please choose:",
|
||||||
|
listOf("Yes", "No"),
|
||||||
|
"clarify-789",
|
||||||
|
sessionId,
|
||||||
|
questionId = "q0",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
advanceUntilIdle()
|
||||||
|
assertEquals(
|
||||||
|
"clarify-789",
|
||||||
|
viewModel.uiState.value.clarifyRequest
|
||||||
|
?.clarifyId,
|
||||||
|
)
|
||||||
|
assertEquals(
|
||||||
|
"q0",
|
||||||
|
viewModel.uiState.value.clarifyRequest
|
||||||
|
?.questionId,
|
||||||
|
)
|
||||||
|
|
||||||
|
viewModel.dismissClarify()
|
||||||
|
advanceUntilIdle()
|
||||||
|
|
||||||
|
assertNull(viewModel.uiState.value.clarifyRequest)
|
||||||
|
verify {
|
||||||
|
HermesWsClient.send(
|
||||||
|
method = WsMethods.CLARIFY_RESPOND,
|
||||||
|
params =
|
||||||
|
mapOf(
|
||||||
|
"session_id" to sessionId,
|
||||||
|
"response" to "The user cancelled — no answer provided.",
|
||||||
|
"answer" to "The user cancelled — no answer provided.",
|
||||||
|
"clarify_id" to "clarify-789",
|
||||||
|
"request_id" to "clarify-789",
|
||||||
|
"question_id" to "q0",
|
||||||
|
),
|
||||||
|
onSent = any(),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// ── Attachments ──────────────────────────────────────────────────────────
|
// ── Attachments ──────────────────────────────────────────────────────────
|
||||||
|
|
||||||
/** Add [count] dummy attachments so a test starts with a populated list. */
|
/** Add [count] dummy attachments so a test starts with a populated list. */
|
||||||
|
|||||||
Reference in new issue
Block a user