fix(chat): preserve midturn steer and session marker ordering (#1241)
This commit is contained in:
1 parent
8cd1ec5298
commit
22fe1290a5
12 files changed
+246
-9
No files matched your search
@@ -53,7 +53,12 @@ interface ChatMessageDao {
|
||||
suspend fun upsert(message: ChatMessageEntity) {
|
||||
val existing = getMessage(message.id)
|
||||
val restId = message.restId ?: existing?.restId
|
||||
val canonicalOrder = canonicalMessageOrder(restId ?: message.id, message.sessionId)
|
||||
val canonicalOrder =
|
||||
if (message.isSessionStartMarker()) {
|
||||
-1L
|
||||
} else {
|
||||
canonicalMessageOrder(restId ?: message.id, message.sessionId)
|
||||
}
|
||||
writeMessage(
|
||||
message.copy(
|
||||
restId = restId,
|
||||
|
||||
@@ -56,6 +56,9 @@ data class ChatMessageEntity(
|
||||
val sortOrder: Long = 0,
|
||||
)
|
||||
|
||||
internal fun ChatMessageEntity.isSessionStartMarker(): Boolean =
|
||||
role == "SYSTEM" && (content == "Session created" || content == "Session branched")
|
||||
|
||||
/** Only the exact session prefix and a nonnegative decimal suffix identify a canonical row. */
|
||||
internal fun canonicalMessageOrder(
|
||||
id: String,
|
||||
|
||||
@@ -39,7 +39,7 @@ fun ChatMessageEntity.toUiModel(): ChatMessage =
|
||||
tps = tps,
|
||||
completionId = completionId,
|
||||
restId = restId,
|
||||
localOrder = sortOrder.takeIf { sortGroup == 1 && it > 0 },
|
||||
localOrder = sortOrder.takeIf { sortGroup == 1 && it > 0 && !isSessionStartMarker() },
|
||||
)
|
||||
|
||||
fun ChatMessage.toEntity(sessionId: String): ChatMessageEntity =
|
||||
|
||||
@@ -36,7 +36,7 @@ private const val MAX_ITERATIONS_SYSTEM_MARKER =
|
||||
*/
|
||||
internal fun ChatMessage.isUserTurnBoundary(): Boolean =
|
||||
role == MessageRole.USER &&
|
||||
displayKind == null &&
|
||||
(displayKind == null || displayKind == "steer") &&
|
||||
!content.startsWith(MAX_ITERATIONS_SYSTEM_MARKER)
|
||||
|
||||
/**
|
||||
|
||||
@@ -164,7 +164,11 @@ internal fun mapServerMessages(
|
||||
// Keep the actual @file:/@image: token here: unlike the dedupe path,
|
||||
// REST-only hydration cannot prove that reference was not user-authored.
|
||||
var finalContent =
|
||||
if (role == MessageRole.USER) stripGatewayAttachedContext(rawContent) else rawContent
|
||||
if (role == MessageRole.USER) {
|
||||
stripGatewayAttachedContext(stripGatewaySteerWrapper(rawContent))
|
||||
} else {
|
||||
rawContent
|
||||
}
|
||||
var attachments: List<Attachment>? = null
|
||||
if (role == MessageRole.ASSISTANT && rawContent.contains("MEDIA:")) {
|
||||
val items = HostMediaExtractor.extract(rawContent)
|
||||
|
||||
@@ -14,6 +14,30 @@ private val CONTEXT_REF_RE =
|
||||
"""@(file|folder|url|image|tool|terminal):(?:"[^"\n]+"|'[^'\n]+'|`[^`\n]+`|\S+)""",
|
||||
)
|
||||
|
||||
private val OUT_OF_BAND_START_RE =
|
||||
Regex("""\[OUT-OF-BAND USER MESSAGE[^\n\]]*\]\s*""", RegexOption.IGNORE_CASE)
|
||||
private val OUT_OF_BAND_END_RE =
|
||||
Regex("""\s*\[/OUT-OF-BAND USER MESSAGE\]""", RegexOption.IGNORE_CASE)
|
||||
|
||||
/**
|
||||
* Remove a gateway-generated `[OUT-OF-BAND USER MESSAGE ...]` wrapper from
|
||||
* mid-turn steering messages while preserving the user-authored prompt.
|
||||
*/
|
||||
internal fun stripGatewaySteerWrapper(content: String): String {
|
||||
val trimmed = content.trimStart()
|
||||
val startMatch = OUT_OF_BAND_START_RE.find(trimmed) ?: return content
|
||||
if (startMatch.range.first != 0) return content
|
||||
val afterStart = trimmed.substring(startMatch.range.last + 1)
|
||||
val endMatch = OUT_OF_BAND_END_RE.find(afterStart)
|
||||
val unwrapped =
|
||||
if (endMatch != null) {
|
||||
afterStart.substring(0, endMatch.range.first)
|
||||
} else {
|
||||
afterStart
|
||||
}
|
||||
return unwrapped.trim()
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove a gateway-generated `--- Attached Context ---` suffix while
|
||||
* preserving the user-authored portion of the message.
|
||||
@@ -231,7 +255,7 @@ internal fun matchTranscriptMessages(
|
||||
* run_agent). Blank lines left behind are dropped too.
|
||||
*/
|
||||
internal fun stripAttachmentRefLines(content: String): String =
|
||||
stripGatewayAttachedContext(content)
|
||||
stripGatewayAttachedContext(stripGatewaySteerWrapper(content))
|
||||
.lines()
|
||||
.map { it.trim() }
|
||||
.filterNot { line ->
|
||||
@@ -276,7 +300,11 @@ internal fun dedupeCachedMessages(
|
||||
}.toMap()
|
||||
return unique.filterNot { it.id in echoes }.map { message ->
|
||||
aliases[message.id]?.let {
|
||||
message.copy(restId = it.canonicalRestId, completionId = message.completionId ?: it.completionId)
|
||||
message.copy(
|
||||
restId = it.canonicalRestId,
|
||||
completionId = message.completionId ?: it.completionId,
|
||||
displayKind = message.displayKind ?: it.displayKind,
|
||||
)
|
||||
} ?: message
|
||||
}
|
||||
}
|
||||
@@ -306,6 +334,7 @@ internal fun mergeCachedTranscriptPage(
|
||||
id = match.id,
|
||||
restId = message.canonicalRestId ?: match.canonicalRestId,
|
||||
completionId = match.completionId ?: message.completionId,
|
||||
displayKind = message.displayKind ?: match.displayKind,
|
||||
)
|
||||
}.toMap()
|
||||
return (
|
||||
@@ -333,6 +362,7 @@ internal fun mergeTranscriptWithLive(
|
||||
match?.role == MessageRole.USER -> {
|
||||
match.copy(
|
||||
restId = (message.canonicalRestId ?: match.canonicalRestId).takeUnless { it == match.id },
|
||||
displayKind = message.displayKind ?: match.displayKind,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -378,8 +408,16 @@ private fun List<ChatMessage>.inTranscriptOrder(): List<ChatMessage> =
|
||||
.thenBy { it.canonicalOrder ?: it.localOrder ?: Long.MAX_VALUE },
|
||||
)
|
||||
|
||||
internal fun ChatMessage.isSessionStartMarker(): Boolean =
|
||||
role == MessageRole.SYSTEM && (content == "Session created" || content == "Session branched")
|
||||
|
||||
private val ChatMessage.canonicalOrder: Long?
|
||||
get() = if (localOrder != null && restId == null) null else canonicalRestId?.substringAfterLast('-')?.toLongOrNull()
|
||||
get() =
|
||||
when {
|
||||
isSessionStartMarker() -> -1L
|
||||
localOrder != null && restId == null -> null
|
||||
else -> canonicalRestId?.substringAfterLast('-')?.toLongOrNull()
|
||||
}
|
||||
|
||||
/** Retain the canonical trace once, without copying it across a user/system/tool boundary. */
|
||||
private fun List<ChatMessage>.reconcileReasoningRows(): List<ChatMessage> =
|
||||
|
||||
@@ -57,6 +57,8 @@ private fun ChatMessage.isSyntheticSystemRow(): Boolean =
|
||||
displayKind == null &&
|
||||
content.startsWith(MAX_ITERATIONS_SYSTEM_MARKER)
|
||||
|
||||
internal fun ChatMessage.isTimelineMarker(): Boolean = displayKind != null && displayKind != "steer"
|
||||
|
||||
/**
|
||||
* Split a flat message list into turns for the full-bleed renderer.
|
||||
*
|
||||
@@ -80,11 +82,12 @@ fun groupIntoTurns(messages: List<ChatMessage>): List<ChatTurn> {
|
||||
// Timeline markers (display_kind) ride as role=user rows but are
|
||||
// NOT user turns — group them as system-style timeline entries so
|
||||
// they render as centered chips, not fake user bubbles (issue #904).
|
||||
// Steer messages (display_kind == "steer") are genuine user turns.
|
||||
// The backend's max-iterations nudge is a role=user row with NO
|
||||
// display_kind (it's stripped on persistence); detect it by its
|
||||
// stable content prefix and route it as a system event too, tagging
|
||||
// it so the timeline chip can name it.
|
||||
message.displayKind != null -> {
|
||||
message.isTimelineMarker() -> {
|
||||
agentEntries += AgentEntry.SystemEvent(message)
|
||||
}
|
||||
|
||||
|
||||
@@ -201,4 +201,24 @@ class ChatMessageMapperTest {
|
||||
|
||||
assertNull(roundTripped.displayKind)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun sessionStartMarkerDoesNotAcquireLocalOrder() {
|
||||
val entity =
|
||||
ChatMessageEntity(
|
||||
id = "uuid-session-created",
|
||||
sessionId = "s",
|
||||
role = "SYSTEM",
|
||||
content = "Session created",
|
||||
timestamp = 1000L,
|
||||
sortGroup = 1,
|
||||
sortOrder = 5L,
|
||||
)
|
||||
|
||||
val ui = entity.toUiModel()
|
||||
|
||||
assertNull(ui.localOrder)
|
||||
assertEquals(MessageRole.SYSTEM, ui.role)
|
||||
assertEquals("Session created", ui.content)
|
||||
}
|
||||
}
|
||||
@@ -91,6 +91,28 @@ class ChatListEntriesTest {
|
||||
assertEquals(mapOf(6 to 5), toolCallMilestoneIndices(messages))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun steerMessageResetsToolCounterAsUserTurnBoundary() {
|
||||
val steer =
|
||||
ChatMessage(
|
||||
role = MessageRole.USER,
|
||||
content = "Stop and run tests",
|
||||
displayKind = "steer",
|
||||
)
|
||||
val messages =
|
||||
listOf(
|
||||
userMessage(),
|
||||
toolMessage(),
|
||||
toolMessage(),
|
||||
steer,
|
||||
toolMessage(),
|
||||
toolMessage(),
|
||||
toolMessage(),
|
||||
)
|
||||
// Only 3 tool calls after the steer boundary, so no 5-call milestone reached
|
||||
assertTrue(toolCallMilestones(messages).isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun syntheticMaxIterationsNudgeDoesNotResetToolCounter() {
|
||||
val nudge =
|
||||
|
||||
@@ -581,6 +581,133 @@ class ChatPagingMergeTest {
|
||||
assertTrue(sameLogicalMessage(canonical.copy(reasoningText = "new trace"), live))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun stripGatewaySteerWrapperRemovesEnvelopesSafely() {
|
||||
val standard =
|
||||
"[OUT-OF-BAND USER MESSAGE — a direct message from the user, delivered once at this position; " +
|
||||
"not tool output and not a new delivery when replayed from conversation history]\n" +
|
||||
"Please use Python instead.\n" +
|
||||
"[/OUT-OF-BAND USER MESSAGE]"
|
||||
assertEquals("Please use Python instead.", stripGatewaySteerWrapper(standard))
|
||||
|
||||
val noDescription =
|
||||
"[OUT-OF-BAND USER MESSAGE]\n" +
|
||||
"Halt.\n" +
|
||||
"[/OUT-OF-BAND USER MESSAGE]"
|
||||
assertEquals("Halt.", stripGatewaySteerWrapper(noDescription))
|
||||
|
||||
val multiline =
|
||||
"[OUT-OF-BAND USER MESSAGE — direct steer]\n" +
|
||||
"First line\n" +
|
||||
"Second line\n" +
|
||||
"[/OUT-OF-BAND USER MESSAGE]"
|
||||
assertEquals("First line\nSecond line", stripGatewaySteerWrapper(multiline))
|
||||
|
||||
val plain = "Normal user message without wrapper"
|
||||
assertEquals(plain, stripGatewaySteerWrapper(plain))
|
||||
|
||||
val interior = "Mentions [OUT-OF-BAND USER MESSAGE] in passing"
|
||||
assertEquals(interior, stripGatewaySteerWrapper(interior))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun midTurnSteerMessageReconcilesAndSortsChronologically() {
|
||||
val userPrompt = ChatMessage(id = "rest-session-0", role = MessageRole.USER, content = "Create a script")
|
||||
val liveSteer =
|
||||
ChatMessage(
|
||||
id = "uuid-steer",
|
||||
role = MessageRole.USER,
|
||||
content = "Use Python instead",
|
||||
)
|
||||
val inFlightAssistant =
|
||||
ChatMessage(
|
||||
id = "uuid-assistant",
|
||||
role = MessageRole.ASSISTANT,
|
||||
content = "Here is the Python script",
|
||||
isStreaming = false,
|
||||
)
|
||||
val current = listOf(userPrompt, inFlightAssistant, liveSteer)
|
||||
|
||||
val serverRows =
|
||||
listOf(
|
||||
SessionMessage(
|
||||
id = 0,
|
||||
role = "user",
|
||||
content = JsonPrimitive("Create a script"),
|
||||
timestamp = JsonPrimitive(0),
|
||||
),
|
||||
SessionMessage(
|
||||
id = 1,
|
||||
role = "user",
|
||||
content =
|
||||
JsonPrimitive(
|
||||
"[OUT-OF-BAND USER MESSAGE — a direct message from the user, " +
|
||||
"delivered once at this position; not tool output and not a new " +
|
||||
"delivery when replayed from conversation history]\n" +
|
||||
"Use Python instead\n" +
|
||||
"[/OUT-OF-BAND USER MESSAGE]",
|
||||
),
|
||||
display_kind = "steer",
|
||||
timestamp = JsonPrimitive(1),
|
||||
),
|
||||
SessionMessage(
|
||||
id = 2,
|
||||
role = "assistant",
|
||||
content = JsonPrimitive("Here is the Python script"),
|
||||
timestamp = JsonPrimitive(2),
|
||||
),
|
||||
)
|
||||
|
||||
val merged = applyServerPage(current, serverRows)
|
||||
|
||||
assertEquals(3, merged.size)
|
||||
assertEquals("rest-session-0", merged[0].id)
|
||||
assertEquals("Create a script", merged[0].content)
|
||||
|
||||
assertEquals("uuid-steer", merged[1].id)
|
||||
assertEquals("rest-session-1", merged[1].canonicalRestId)
|
||||
assertEquals("Use Python instead", merged[1].content)
|
||||
assertEquals("steer", merged[1].displayKind)
|
||||
|
||||
assertEquals("rest-session-2", merged[2].canonicalRestId)
|
||||
assertEquals("Here is the Python script", merged[2].content)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun sessionCreatedMarkerStaysAtStartOfTranscript() {
|
||||
val sessionCreated =
|
||||
ChatMessage(
|
||||
id = "uuid-sys-start",
|
||||
role = MessageRole.SYSTEM,
|
||||
content = "Session created",
|
||||
)
|
||||
val current = listOf(sessionCreated)
|
||||
|
||||
val serverRows =
|
||||
listOf(
|
||||
SessionMessage(
|
||||
id = 0,
|
||||
role = "user",
|
||||
content = JsonPrimitive("Hello"),
|
||||
timestamp = JsonPrimitive(1),
|
||||
),
|
||||
SessionMessage(
|
||||
id = 1,
|
||||
role = "assistant",
|
||||
content = JsonPrimitive("Hi there!"),
|
||||
timestamp = JsonPrimitive(2),
|
||||
),
|
||||
)
|
||||
|
||||
val merged = applyServerPage(current, serverRows)
|
||||
|
||||
assertEquals(3, merged.size)
|
||||
assertEquals("uuid-sys-start", merged[0].id)
|
||||
assertEquals("Session created", merged[0].content)
|
||||
assertEquals("rest-session-0", merged[1].canonicalRestId)
|
||||
assertEquals("rest-session-1", merged[2].canonicalRestId)
|
||||
}
|
||||
|
||||
private fun assertBoundaryTranscript(messages: List<ChatMessage>) {
|
||||
assertEquals(listOf("rest-session-100", "rest-session-200"), messages.map { it.canonicalRestId })
|
||||
assertEquals(listOf("boundary trace", ""), messages.map { it.reasoningText })
|
||||
|
||||
@@ -94,6 +94,15 @@ class FullBleedTurnsTest {
|
||||
assertEquals(listOf<ChatTurn>(entries(AgentEntry.SystemEvent(marker))), result)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `steer message with display_kind steer is a User turn not a SystemEvent entry`() {
|
||||
val steer =
|
||||
msg("u1", MessageRole.USER, content = "Stop and run tests")
|
||||
.copy(displayKind = "steer")
|
||||
val result = groupIntoTurns(listOf(steer))
|
||||
assertEquals(listOf<ChatTurn>(ChatTurn.User(steer)), 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
|
||||
|
||||
@@ -3,6 +3,7 @@ package com.m57.hermescontrol.ui.chat.fakes
|
||||
import com.m57.hermescontrol.data.local.ChatMessageDao
|
||||
import com.m57.hermescontrol.data.local.ChatMessageEntity
|
||||
import com.m57.hermescontrol.data.local.canonicalMessageOrder
|
||||
import com.m57.hermescontrol.data.local.isSessionStartMarker
|
||||
import java.util.concurrent.ConcurrentHashMap
|
||||
import java.util.concurrent.ConcurrentMap
|
||||
|
||||
@@ -86,7 +87,12 @@ class FakeChatMessageDao : ChatMessageDao {
|
||||
fun addMessageDirect(message: ChatMessageEntity) {
|
||||
val existing = messages[message.id]
|
||||
if (existing == null) insertionSequence++
|
||||
val order = canonicalMessageOrder(message.restId ?: message.id, message.sessionId)
|
||||
val order =
|
||||
if (message.isSessionStartMarker()) {
|
||||
-1L
|
||||
} else {
|
||||
canonicalMessageOrder(message.restId ?: message.id, message.sessionId)
|
||||
}
|
||||
messages[message.id] =
|
||||
message.copy(
|
||||
sortGroup = if (order != null) 0 else 1,
|
||||
|
||||
Reference in new issue
Block a user