fix(chat): anchor local commands, feedback, and clarify replies in transcript (#1241)
This commit is contained in:
@@ -69,6 +69,7 @@ class ChatClarifyDelegate(
|
|||||||
ChatMessage(
|
ChatMessage(
|
||||||
role = MessageRole.USER,
|
role = MessageRole.USER,
|
||||||
content = displayContent,
|
content = displayContent,
|
||||||
|
displayKind = "clarify_response",
|
||||||
)
|
)
|
||||||
|
|
||||||
uiState.update { state ->
|
uiState.update { state ->
|
||||||
|
|||||||
@@ -36,7 +36,7 @@ private const val MAX_ITERATIONS_SYSTEM_MARKER =
|
|||||||
*/
|
*/
|
||||||
internal fun ChatMessage.isUserTurnBoundary(): Boolean =
|
internal fun ChatMessage.isUserTurnBoundary(): Boolean =
|
||||||
role == MessageRole.USER &&
|
role == MessageRole.USER &&
|
||||||
(displayKind == null || displayKind == "steer") &&
|
(displayKind == null || displayKind == "steer" || displayKind == "clarify_response") &&
|
||||||
!content.startsWith(MAX_ITERATIONS_SYSTEM_MARKER)
|
!content.startsWith(MAX_ITERATIONS_SYSTEM_MARKER)
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -171,6 +171,7 @@ internal class TranscriptComparison(
|
|||||||
}
|
}
|
||||||
val ta = trimmed.getOrPut(a.content) { a.content.trim() }
|
val ta = trimmed.getOrPut(a.content) { a.content.trim() }
|
||||||
val tb = trimmed.getOrPut(b.content) { b.content.trim() }
|
val tb = trimmed.getOrPut(b.content) { b.content.trim() }
|
||||||
|
if (ta.startsWith("/") || tb.startsWith("/")) return false
|
||||||
if (ta == tb) return true
|
if (ta == tb) return true
|
||||||
if (a.role == MessageRole.USER &&
|
if (a.role == MessageRole.USER &&
|
||||||
captions.getOrPut(ta) { stripAttachmentRefLines(ta) } ==
|
captions.getOrPut(ta) { stripAttachmentRefLines(ta) } ==
|
||||||
@@ -345,7 +346,7 @@ internal fun mergeCachedTranscriptPage(
|
|||||||
return (
|
return (
|
||||||
current.map { replacements[it.id] ?: it } +
|
current.map { replacements[it.id] ?: it } +
|
||||||
incoming.filterIndexed { index, _ -> matches[index] == null }
|
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. */
|
/** Merge one page with the current snapshot without consuming repeated results more than once. */
|
||||||
@@ -412,7 +413,7 @@ internal fun mergeTranscriptWithLive(
|
|||||||
.reconcileReasoningRows()
|
.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(
|
private fun List<ChatMessage>.inTranscriptOrder(
|
||||||
previous: List<ChatMessage>,
|
previous: List<ChatMessage>,
|
||||||
resolvedOrders: Map<String, Long>,
|
resolvedOrders: Map<String, Long>,
|
||||||
@@ -421,39 +422,64 @@ private fun List<ChatMessage>.inTranscriptOrder(
|
|||||||
var precedingCanonical: Long? = null
|
var precedingCanonical: Long? = null
|
||||||
var hasPendingPredecessor = false
|
var hasPendingPredecessor = false
|
||||||
var pendingLocalOrder: Long? = null
|
var pendingLocalOrder: Long? = null
|
||||||
val noticeAnchors = mutableMapOf<String, Long>()
|
val localAnchors = mutableMapOf<String, Long>()
|
||||||
val pendingOrderByNotice = mutableMapOf<String, Long>()
|
val pendingOrderByLocal = mutableMapOf<String, Long>()
|
||||||
previous.forEach { message ->
|
previous.forEach { message ->
|
||||||
val order = resolvedOrders[message.id] ?: message.canonicalOrder
|
val order = resolvedOrders[message.id] ?: message.canonicalOrder
|
||||||
if (order != null) {
|
if (order != null) {
|
||||||
precedingCanonical = order
|
precedingCanonical = order
|
||||||
hasPendingPredecessor = false
|
hasPendingPredecessor = false
|
||||||
pendingLocalOrder = null
|
pendingLocalOrder = null
|
||||||
} else if (message.role != MessageRole.SYSTEM) {
|
} else if (message.isPermanentlyLocal()) {
|
||||||
hasPendingPredecessor = true
|
localAnchors[message.id] =
|
||||||
pendingLocalOrder = message.localOrder
|
|
||||||
} else {
|
|
||||||
noticeAnchors[message.id] =
|
|
||||||
if (hasPendingPredecessor) {
|
if (hasPendingPredecessor) {
|
||||||
Long.MAX_VALUE
|
Long.MAX_VALUE
|
||||||
} else {
|
} else {
|
||||||
precedingCanonical?.takeIf { it >= 0L }
|
precedingCanonical?.takeIf { it >= 0L }
|
||||||
?: latestCanonical
|
?: 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 }
|
val previousIndices = previous.withIndex().associate { it.value.id to it.index }
|
||||||
return withIndex()
|
return withIndex()
|
||||||
.sortedWith(
|
.sortedWith(
|
||||||
compareBy<IndexedValue<ChatMessage>> {
|
compareBy<IndexedValue<ChatMessage>> {
|
||||||
it.value.canonicalOrder ?: noticeAnchors[it.value.id] ?: Long.MAX_VALUE
|
it.value.canonicalOrder ?: localAnchors[it.value.id] ?: Long.MAX_VALUE
|
||||||
}.thenBy { if (noticeAnchors[it.value.id]?.let { anchor -> anchor != Long.MAX_VALUE } == true) 1 else 0 }
|
}.thenBy { if (localAnchors[it.value.id]?.let { anchor -> anchor != Long.MAX_VALUE } == true) 1 else 0 }
|
||||||
.thenBy { it.value.localOrder ?: pendingOrderByNotice[it.value.id] ?: Long.MAX_VALUE }
|
.thenBy { it.value.localOrder ?: pendingOrderByLocal[it.value.id] ?: Long.MAX_VALUE }
|
||||||
.thenBy { previousIndices[it.value.id] ?: it.index },
|
.thenBy { previousIndices[it.value.id] ?: it.index },
|
||||||
).map { it.value }
|
).map { it.value }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
internal fun ChatMessage.isPermanentlyLocal(): Boolean =
|
||||||
|
role == MessageRole.SYSTEM ||
|
||||||
|
(role == MessageRole.USER && (content.startsWith("/") || displayKind == "clarify_response")) ||
|
||||||
|
(role == MessageRole.ASSISTANT && canonicalRestId == null && isLocalFeedbackRow(this))
|
||||||
|
|
||||||
|
private fun isLocalFeedbackRow(message: ChatMessage): Boolean =
|
||||||
|
message.localOrder != null || isLocalFeedbackContent(message.content)
|
||||||
|
|
||||||
|
private fun isLocalFeedbackContent(content: String): Boolean =
|
||||||
|
content.endsWith(" is not supported on mobile") ||
|
||||||
|
content.startsWith("usage: /queue") ||
|
||||||
|
content.startsWith("reasoning: ") ||
|
||||||
|
content.startsWith("Could not read reasoning status:") ||
|
||||||
|
content == "Reasoning is not supported for the current model." ||
|
||||||
|
content == "Reasoning cannot be disabled for this model (always on)." ||
|
||||||
|
content.startsWith("Could not change reasoning:") ||
|
||||||
|
content == "Reasoning controls require an active session." ||
|
||||||
|
content == "No active session. Use `/new` to create one." ||
|
||||||
|
content == "No active session to undo." ||
|
||||||
|
content == "No active session for side questions. Start a chat first." ||
|
||||||
|
content.startsWith("Bot chats are one continuous conversation — compacting instead.") ||
|
||||||
|
content.startsWith("⚠ ") ||
|
||||||
|
(content.startsWith("/") && content.contains(": ")) ||
|
||||||
|
content == "Failed to undo."
|
||||||
|
|
||||||
internal fun ChatMessage.isSessionStartMarker(): Boolean =
|
internal fun ChatMessage.isSessionStartMarker(): Boolean =
|
||||||
role == MessageRole.SYSTEM && (content == "Session created" || content == "Session branched")
|
role == MessageRole.SYSTEM && (content == "Session created" || content == "Session branched")
|
||||||
|
|
||||||
|
|||||||
@@ -57,7 +57,8 @@ private fun ChatMessage.isSyntheticSystemRow(): Boolean =
|
|||||||
displayKind == null &&
|
displayKind == null &&
|
||||||
content.startsWith(MAX_ITERATIONS_SYSTEM_MARKER)
|
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"
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Split a flat message list into turns for the full-bleed renderer.
|
* Split a flat message list into turns for the full-bleed renderer.
|
||||||
|
|||||||
@@ -748,6 +748,151 @@ class ChatPagingMergeTest {
|
|||||||
assertEquals(listOf("rest-s-1", "live-reply", "review", "rest-s-3"), synced.map { it.id })
|
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 = "reasoning: low · display on")
|
||||||
|
|
||||||
|
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
|
@Test
|
||||||
fun sessionCreatedMarkerStaysAtStartOfTranscript() {
|
fun sessionCreatedMarkerStaysAtStartOfTranscript() {
|
||||||
val sessionCreated =
|
val sessionCreated =
|
||||||
|
|||||||
@@ -700,6 +700,86 @@ class ChatViewModelTest {
|
|||||||
verify { HermesWsClient.send(WsMethods.SESSION_INTERRUPT, any(), any()) }
|
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
|
@Test
|
||||||
fun testSlashCommand_interrupt_sendsInterrupt() =
|
fun testSlashCommand_interrupt_sendsInterrupt() =
|
||||||
runTest {
|
runTest {
|
||||||
|
|||||||
Reference in New Issue
Block a user