Merge pull request #1249 from Hy4ri/fix/local-chat-event-ordering
fix(chat): anchor local commands, feedback, and clarify replies in transcript (#1241)
This commit is contained in:
8 files changed
+329
-16
No files matched your search
@@ -69,6 +69,7 @@ class ChatClarifyDelegate(
|
||||
ChatMessage(
|
||||
role = MessageRole.USER,
|
||||
content = displayContent,
|
||||
displayKind = "clarify_response",
|
||||
)
|
||||
|
||||
uiState.update { state ->
|
||||
|
||||
@@ -36,7 +36,7 @@ private const val MAX_ITERATIONS_SYSTEM_MARKER =
|
||||
*/
|
||||
internal fun ChatMessage.isUserTurnBoundary(): Boolean =
|
||||
role == MessageRole.USER &&
|
||||
(displayKind == null || displayKind == "steer") &&
|
||||
(displayKind == null || displayKind == "steer" || displayKind == "clarify_response") &&
|
||||
!content.startsWith(MAX_ITERATIONS_SYSTEM_MARKER)
|
||||
|
||||
/**
|
||||
|
||||
@@ -169,8 +169,21 @@ internal class TranscriptComparison(
|
||||
val key = toolKey(a.content)
|
||||
return key != null && key == toolKey(b.content)
|
||||
}
|
||||
// Rows generated only by Mobile never have a canonical REST counterpart.
|
||||
// Do not let coincidentally identical server text claim their identity.
|
||||
if (a.role == MessageRole.ASSISTANT &&
|
||||
(a.displayKind == "local_feedback" || b.displayKind == "local_feedback")
|
||||
) {
|
||||
return false
|
||||
}
|
||||
if (a.role == MessageRole.USER &&
|
||||
(a.displayKind == "clarify_response" || b.displayKind == "clarify_response")
|
||||
) {
|
||||
return false
|
||||
}
|
||||
val ta = trimmed.getOrPut(a.content) { a.content.trim() }
|
||||
val tb = trimmed.getOrPut(b.content) { b.content.trim() }
|
||||
if (a.role == MessageRole.USER && (ta.startsWith("/") || tb.startsWith("/"))) return false
|
||||
if (ta == tb) return true
|
||||
if (a.role == MessageRole.USER &&
|
||||
captions.getOrPut(ta) { stripAttachmentRefLines(ta) } ==
|
||||
@@ -345,7 +358,7 @@ internal fun mergeCachedTranscriptPage(
|
||||
return (
|
||||
current.map { replacements[it.id] ?: it } +
|
||||
incoming.filterIndexed { index, _ -> matches[index] == null }
|
||||
).dedupeById().inTranscriptOrder(current, resolvedOrders).reconcileReasoningRows()
|
||||
).dedupeById().inTranscriptOrder(incoming + current, resolvedOrders).reconcileReasoningRows()
|
||||
}
|
||||
|
||||
/** Merge one page with the current snapshot without consuming repeated results more than once. */
|
||||
@@ -412,7 +425,7 @@ internal fun mergeTranscriptWithLive(
|
||||
.reconcileReasoningRows()
|
||||
}
|
||||
|
||||
/** Keep server order; place local notices after their last preceding confirmed message, not at the transcript tail. */
|
||||
/** Keep server order; place local notices and commands after their last preceding confirmed message, not at the transcript tail. */
|
||||
private fun List<ChatMessage>.inTranscriptOrder(
|
||||
previous: List<ChatMessage>,
|
||||
resolvedOrders: Map<String, Long>,
|
||||
@@ -421,39 +434,44 @@ private fun List<ChatMessage>.inTranscriptOrder(
|
||||
var precedingCanonical: Long? = null
|
||||
var hasPendingPredecessor = false
|
||||
var pendingLocalOrder: Long? = null
|
||||
val noticeAnchors = mutableMapOf<String, Long>()
|
||||
val pendingOrderByNotice = mutableMapOf<String, Long>()
|
||||
val localAnchors = mutableMapOf<String, Long>()
|
||||
val pendingOrderByLocal = mutableMapOf<String, Long>()
|
||||
previous.forEach { message ->
|
||||
val order = resolvedOrders[message.id] ?: message.canonicalOrder
|
||||
if (order != null) {
|
||||
precedingCanonical = order
|
||||
hasPendingPredecessor = false
|
||||
pendingLocalOrder = null
|
||||
} else if (message.role != MessageRole.SYSTEM) {
|
||||
hasPendingPredecessor = true
|
||||
pendingLocalOrder = message.localOrder
|
||||
} else {
|
||||
noticeAnchors[message.id] =
|
||||
} else if (message.isPermanentlyLocal()) {
|
||||
localAnchors[message.id] =
|
||||
if (hasPendingPredecessor) {
|
||||
Long.MAX_VALUE
|
||||
} else {
|
||||
precedingCanonical?.takeIf { it >= 0L }
|
||||
?: latestCanonical
|
||||
}
|
||||
if (hasPendingPredecessor) pendingOrderByNotice[message.id] = pendingLocalOrder ?: Long.MAX_VALUE
|
||||
if (hasPendingPredecessor) pendingOrderByLocal[message.id] = pendingLocalOrder ?: Long.MAX_VALUE
|
||||
} else {
|
||||
hasPendingPredecessor = true
|
||||
pendingLocalOrder = message.localOrder
|
||||
}
|
||||
}
|
||||
val previousIndices = previous.withIndex().associate { it.value.id to it.index }
|
||||
return withIndex()
|
||||
.sortedWith(
|
||||
compareBy<IndexedValue<ChatMessage>> {
|
||||
it.value.canonicalOrder ?: noticeAnchors[it.value.id] ?: Long.MAX_VALUE
|
||||
}.thenBy { if (noticeAnchors[it.value.id]?.let { anchor -> anchor != Long.MAX_VALUE } == true) 1 else 0 }
|
||||
.thenBy { it.value.localOrder ?: pendingOrderByNotice[it.value.id] ?: Long.MAX_VALUE }
|
||||
it.value.canonicalOrder ?: localAnchors[it.value.id] ?: Long.MAX_VALUE
|
||||
}.thenBy { if (localAnchors[it.value.id]?.let { anchor -> anchor != Long.MAX_VALUE } == true) 1 else 0 }
|
||||
.thenBy { it.value.localOrder ?: pendingOrderByLocal[it.value.id] ?: Long.MAX_VALUE }
|
||||
.thenBy { previousIndices[it.value.id] ?: it.index },
|
||||
).map { it.value }
|
||||
}
|
||||
|
||||
internal fun ChatMessage.isPermanentlyLocal(): Boolean =
|
||||
role == MessageRole.SYSTEM ||
|
||||
(role == MessageRole.USER && (content.startsWith("/") || displayKind == "clarify_response")) ||
|
||||
(role == MessageRole.ASSISTANT && displayKind == "local_feedback")
|
||||
|
||||
internal fun ChatMessage.isSessionStartMarker(): Boolean =
|
||||
role == MessageRole.SYSTEM && (content == "Session created" || content == "Session branched")
|
||||
|
||||
|
||||
@@ -2655,7 +2655,12 @@ class ChatViewModel(
|
||||
}
|
||||
|
||||
private fun addAssistantMessage(text: String) {
|
||||
val msg = ChatMessage(role = MessageRole.ASSISTANT, content = text)
|
||||
val msg =
|
||||
ChatMessage(
|
||||
role = MessageRole.ASSISTANT,
|
||||
content = text,
|
||||
displayKind = "local_feedback",
|
||||
)
|
||||
_uiState.update { it.copy(messages = it.messages + msg) }
|
||||
|
||||
// Persist — OUTSIDE update{}
|
||||
|
||||
@@ -57,7 +57,11 @@ private fun ChatMessage.isSyntheticSystemRow(): Boolean =
|
||||
displayKind == null &&
|
||||
content.startsWith(MAX_ITERATIONS_SYSTEM_MARKER)
|
||||
|
||||
internal fun ChatMessage.isTimelineMarker(): Boolean = displayKind != null && displayKind != "steer"
|
||||
internal fun ChatMessage.isTimelineMarker(): Boolean =
|
||||
displayKind != null &&
|
||||
displayKind != "steer" &&
|
||||
displayKind != "clarify_response" &&
|
||||
displayKind != "local_feedback"
|
||||
|
||||
/**
|
||||
* Split a flat message list into turns for the full-bleed renderer.
|
||||
|
||||
@@ -581,6 +581,51 @@ class ChatPagingMergeTest {
|
||||
assertTrue(sameLogicalMessage(canonical.copy(reasoningText = "new trace"), live))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun assistantReplyBeginningWithSlashStillMatchesCanonicalEcho() {
|
||||
val live =
|
||||
ChatMessage(
|
||||
id = "uuid-answer",
|
||||
role = MessageRole.ASSISTANT,
|
||||
content = "/help is the command you're looking for.",
|
||||
)
|
||||
val canonical =
|
||||
live.copy(
|
||||
id = "rest-session-10",
|
||||
restId = null,
|
||||
)
|
||||
|
||||
assertTrue(sameLogicalMessage(canonical, live))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun permanentlyLocalRowsDoNotMatchCanonicalRowsByContent() {
|
||||
val localFeedback =
|
||||
ChatMessage(
|
||||
id = "uuid-feedback",
|
||||
role = MessageRole.ASSISTANT,
|
||||
content = "Command completed successfully",
|
||||
displayKind = "local_feedback",
|
||||
)
|
||||
val canonicalAssistant =
|
||||
ChatMessage(
|
||||
id = "rest-session-10",
|
||||
role = MessageRole.ASSISTANT,
|
||||
content = "Command completed successfully",
|
||||
)
|
||||
val clarify =
|
||||
ChatMessage(
|
||||
id = "uuid-clarify",
|
||||
role = MessageRole.USER,
|
||||
content = "yes",
|
||||
displayKind = "clarify_response",
|
||||
)
|
||||
val canonicalUser = ChatMessage(id = "rest-session-11", role = MessageRole.USER, content = "yes")
|
||||
|
||||
assertTrue(!sameLogicalMessage(canonicalAssistant, localFeedback))
|
||||
assertTrue(!sameLogicalMessage(canonicalUser, clarify))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun stripGatewaySteerWrapperRemovesEnvelopesSafely() {
|
||||
val standard =
|
||||
@@ -748,6 +793,156 @@ class ChatPagingMergeTest {
|
||||
assertEquals(listOf("rest-s-1", "live-reply", "review", "rest-s-3"), synced.map { it.id })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun stopCommandAndSessionInterruptedStayInPlaceAcrossFutureSyncs() {
|
||||
val prompt = ChatMessage(id = "rest-session-0", role = MessageRole.USER, content = "long running task")
|
||||
val partialAssistant =
|
||||
ChatMessage(id = "rest-session-1", role = MessageRole.ASSISTANT, content = "Working on it...")
|
||||
val stopCommand = ChatMessage(id = "uuid-stop", role = MessageRole.USER, content = "/stop")
|
||||
val interrupted =
|
||||
ChatMessage(id = "uuid-interrupted", role = MessageRole.SYSTEM, content = "Session interrupted")
|
||||
|
||||
val current = listOf(prompt, partialAssistant, stopCommand, interrupted)
|
||||
|
||||
// Now later turn arrives from server: user asks something else, assistant answers
|
||||
val serverRows =
|
||||
listOf(
|
||||
SessionMessage(
|
||||
id = 0,
|
||||
role = "user",
|
||||
content = JsonPrimitive("long running task"),
|
||||
timestamp = JsonPrimitive(1),
|
||||
),
|
||||
SessionMessage(
|
||||
id = 1,
|
||||
role = "assistant",
|
||||
content = JsonPrimitive("Working on it..."),
|
||||
timestamp = JsonPrimitive(2),
|
||||
),
|
||||
SessionMessage(
|
||||
id = 2,
|
||||
role = "user",
|
||||
content = JsonPrimitive("next question"),
|
||||
timestamp = JsonPrimitive(3),
|
||||
),
|
||||
SessionMessage(
|
||||
id = 3,
|
||||
role = "assistant",
|
||||
content = JsonPrimitive("next answer"),
|
||||
timestamp = JsonPrimitive(4),
|
||||
),
|
||||
)
|
||||
|
||||
val merged = applyServerPage(current, serverRows)
|
||||
|
||||
assertEquals(
|
||||
listOf(
|
||||
"rest-session-0",
|
||||
"rest-session-1",
|
||||
"uuid-stop",
|
||||
"uuid-interrupted",
|
||||
"rest-session-2",
|
||||
"rest-session-3",
|
||||
),
|
||||
merged.map { it.canonicalRestId ?: it.id },
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun commandEchoAndOutputStayInPlaceAcrossFutureSyncs() {
|
||||
val prompt = ChatMessage(id = "rest-session-0", role = MessageRole.USER, content = "what model are you?")
|
||||
val answer = ChatMessage(id = "rest-session-1", role = MessageRole.ASSISTANT, content = "I am Hermes.")
|
||||
val command = ChatMessage(id = "uuid-cmd", role = MessageRole.USER, content = "/reasoning")
|
||||
val output =
|
||||
ChatMessage(
|
||||
id = "uuid-output",
|
||||
role = MessageRole.ASSISTANT,
|
||||
content = "Command completed successfully",
|
||||
displayKind = "local_feedback",
|
||||
)
|
||||
|
||||
val current = listOf(prompt, answer, command, output)
|
||||
|
||||
val serverRows =
|
||||
listOf(
|
||||
SessionMessage(
|
||||
id = 0,
|
||||
role = "user",
|
||||
content = JsonPrimitive("what model are you?"),
|
||||
timestamp = JsonPrimitive(1),
|
||||
),
|
||||
SessionMessage(
|
||||
id = 1,
|
||||
role = "assistant",
|
||||
content = JsonPrimitive("I am Hermes."),
|
||||
timestamp = JsonPrimitive(2),
|
||||
),
|
||||
SessionMessage(
|
||||
id = 2,
|
||||
role = "user",
|
||||
content = JsonPrimitive("next question"),
|
||||
timestamp = JsonPrimitive(3),
|
||||
),
|
||||
SessionMessage(
|
||||
id = 3,
|
||||
role = "assistant",
|
||||
content = JsonPrimitive("next answer"),
|
||||
timestamp = JsonPrimitive(4),
|
||||
),
|
||||
)
|
||||
|
||||
val merged = applyServerPage(current, serverRows)
|
||||
|
||||
assertEquals(
|
||||
listOf("rest-session-0", "rest-session-1", "uuid-cmd", "uuid-output", "rest-session-2", "rest-session-3"),
|
||||
merged.map { it.canonicalRestId ?: it.id },
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun clarifyAnswerStaysInPlaceAcrossFutureSyncs() {
|
||||
val prompt = ChatMessage(id = "rest-session-0", role = MessageRole.USER, content = "deploy this")
|
||||
val assistant = ChatMessage(id = "rest-session-1", role = MessageRole.ASSISTANT, content = "Where?")
|
||||
val clarifyAnswer =
|
||||
ChatMessage(
|
||||
id = "uuid-clarify",
|
||||
role = MessageRole.USER,
|
||||
content = "production",
|
||||
displayKind = "clarify_response",
|
||||
)
|
||||
|
||||
val current = listOf(prompt, assistant, clarifyAnswer)
|
||||
|
||||
val serverRows =
|
||||
listOf(
|
||||
SessionMessage(
|
||||
id = 0,
|
||||
role = "user",
|
||||
content = JsonPrimitive("deploy this"),
|
||||
timestamp = JsonPrimitive(1),
|
||||
),
|
||||
SessionMessage(
|
||||
id = 1,
|
||||
role = "assistant",
|
||||
content = JsonPrimitive("Where?"),
|
||||
timestamp = JsonPrimitive(2),
|
||||
),
|
||||
SessionMessage(
|
||||
id = 2,
|
||||
role = "assistant",
|
||||
content = JsonPrimitive("Deploying to production now..."),
|
||||
timestamp = JsonPrimitive(3),
|
||||
),
|
||||
)
|
||||
|
||||
val merged = applyServerPage(current, serverRows)
|
||||
|
||||
assertEquals(
|
||||
listOf("rest-session-0", "rest-session-1", "uuid-clarify", "rest-session-2"),
|
||||
merged.map { it.canonicalRestId ?: it.id },
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun sessionCreatedMarkerStaysAtStartOfTranscript() {
|
||||
val sessionCreated =
|
||||
|
||||
@@ -700,6 +700,86 @@ class ChatViewModelTest {
|
||||
verify { HermesWsClient.send(WsMethods.SESSION_INTERRUPT, any(), any()) }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testSlashCommand_stop_andInterruptedNoticeStayInPlaceAcrossSync() =
|
||||
runTest {
|
||||
val (viewModel, sessionId) = createViewModelWithSession()
|
||||
|
||||
// 1. Initial messages in UI state
|
||||
val m0 = ChatMessage(id = "rest-$sessionId-0", role = MessageRole.USER, content = "start task")
|
||||
val m1 = ChatMessage(id = "rest-$sessionId-1", role = MessageRole.ASSISTANT, content = "running...")
|
||||
val stateField =
|
||||
ChatViewModel::class.java
|
||||
.getDeclaredField("_uiState")
|
||||
.apply { isAccessible = true }
|
||||
|
||||
@Suppress("UNCHECKED_CAST")
|
||||
val state = stateField.get(viewModel) as MutableStateFlow<ChatUiState>
|
||||
state.value = viewModel.uiState.value.copy(messages = listOf(m0, m1))
|
||||
|
||||
every {
|
||||
HermesWsClient.send(WsMethods.SESSION_INTERRUPT, any(), any())
|
||||
} answers {
|
||||
val onSent = thirdArg<((String) -> Unit)?>()
|
||||
onSent?.invoke("interrupt-req-1")
|
||||
"interrupt-req-1"
|
||||
}
|
||||
|
||||
// 2. User sends /stop
|
||||
viewModel.sendMessage("/stop")
|
||||
advanceUntilIdle()
|
||||
|
||||
// 3. WS returns interrupt success -> adds "Session interrupted"
|
||||
mockEventsFlow.emit(WsEvent.RpcResult("interrupt-req-1", mapOf("ok" to true)))
|
||||
advanceUntilIdle()
|
||||
|
||||
// 4. Server transcript sync arrives with newer turns
|
||||
val serverRows =
|
||||
listOf(
|
||||
com.m57.hermescontrol.data.model.SessionMessage(
|
||||
id = 0,
|
||||
role = "user",
|
||||
content = kotlinx.serialization.json.JsonPrimitive("start task"),
|
||||
timestamp = kotlinx.serialization.json.JsonPrimitive(1),
|
||||
),
|
||||
com.m57.hermescontrol.data.model.SessionMessage(
|
||||
id = 1,
|
||||
role = "assistant",
|
||||
content = kotlinx.serialization.json.JsonPrimitive("running..."),
|
||||
timestamp = kotlinx.serialization.json.JsonPrimitive(2),
|
||||
),
|
||||
com.m57.hermescontrol.data.model.SessionMessage(
|
||||
id = 2,
|
||||
role = "user",
|
||||
content = kotlinx.serialization.json.JsonPrimitive("next prompt"),
|
||||
timestamp = kotlinx.serialization.json.JsonPrimitive(3),
|
||||
),
|
||||
com.m57.hermescontrol.data.model.SessionMessage(
|
||||
id = 3,
|
||||
role = "assistant",
|
||||
content = kotlinx.serialization.json.JsonPrimitive("next reply"),
|
||||
timestamp = kotlinx.serialization.json.JsonPrimitive(4),
|
||||
),
|
||||
)
|
||||
|
||||
// Simulate sync/merge with serverRows
|
||||
val current = viewModel.uiState.value.messages
|
||||
val merged =
|
||||
com.m57.hermescontrol.ui.chat.mergeTranscriptWithLive(
|
||||
com.m57.hermescontrol.ui.chat
|
||||
.mapServerMessages(sessionId, serverRows, 0, true, current),
|
||||
current,
|
||||
chronological = true,
|
||||
preserveLiveIds = true,
|
||||
)
|
||||
|
||||
val contents = merged.map { it.content }
|
||||
assertEquals(
|
||||
listOf("start task", "running...", "/stop", "Session interrupted", "next prompt", "next reply"),
|
||||
contents,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testSlashCommand_interrupt_sendsInterrupt() =
|
||||
runTest {
|
||||
|
||||
@@ -103,6 +103,16 @@ class FullBleedTurnsTest {
|
||||
assertEquals(listOf<ChatTurn>(ChatTurn.User(steer)), result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `local feedback display kind remains assistant prose`() {
|
||||
val feedback =
|
||||
msg("a1", MessageRole.ASSISTANT, content = "Command completed successfully")
|
||||
.copy(displayKind = "local_feedback")
|
||||
val result = groupIntoTurns(listOf(feedback))
|
||||
|
||||
assertEquals(listOf<ChatTurn>(entries(AgentEntry.Prose(feedback))), result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `max-iterations nudge is a SystemEvent not a user bubble`() {
|
||||
// Backend persists the max-iterations notice as a plain role=user row
|
||||
|
||||
Reference in new issue
Block a user