refactor(chat): single owner for display kinds and REST row ids (#1364)

app/src/main/java/com/m57/hermescontrol/data/local/ChatMessageEntity.kt

@@ -66,7 +66,9 @@ internal fun canonicalMessageOrder(
     id: String,
     sessionId: String,
 ): Long? {
-    val prefix = "rest-$sessionId-"
+    val prefix =
+        com.m57.hermescontrol.ui.chat.RestMessageId
+            .sessionPrefix(sessionId)
     if (!id.startsWith(prefix)) return null
     val suffix = id.removePrefix(prefix)
     return suffix.takeIf { it.isNotEmpty() && it.all { char -> char in '0'..'9' } }?.toLongOrNull()

app/src/main/java/com/m57/hermescontrol/ui/chat/ChatClarifyDelegate.kt

@@ -84,7 +84,7 @@ class ChatClarifyDelegate(
             ChatMessage(
                 role = MessageRole.USER,
                 content = displayContent,
-                displayKind = "clarify_response",
+                displayKind = DisplayKind.CLARIFY_RESPONSE,
             )
 
         uiState.update { state ->

app/src/main/java/com/m57/hermescontrol/ui/chat/ChatListEntries.kt

@@ -36,7 +36,7 @@ private const val MAX_ITERATIONS_SYSTEM_MARKER =
  */
 internal fun ChatMessage.isUserTurnBoundary(): Boolean =
     role == MessageRole.USER &&
-        (displayKind == null || displayKind == "steer" || displayKind == "clarify_response") &&
+        (displayKind == null || displayKind == DisplayKind.STEER || displayKind == DisplayKind.CLARIFY_RESPONSE) &&
         !content.startsWith(MAX_ITERATIONS_SYSTEM_MARKER)
 
 /**

app/src/main/java/com/m57/hermescontrol/ui/chat/ChatMessage.kt

@@ -117,7 +117,7 @@ data class ChatMessage(
 
 /** Cached REST rows already carry their canonical identity in the persisted primary key. */
 internal val ChatMessage.canonicalRestId: String?
-    get() = restId ?: id.takeIf { it.startsWith("rest-") }
+    get() = restId ?: id.takeIf(RestMessageId::isRest)
 
 /** A persisted RUNNING snapshot is not evidence of current tool activity. */
 internal val ChatMessage.isToolRunning: Boolean

app/src/main/java/com/m57/hermescontrol/ui/chat/ChatMessageIdentity.kt

@@ -0,0 +1,40 @@
+package com.m57.hermescontrol.ui.chat
+
+/**
+ * Known `display_kind` wire values (#1336). [ChatMessage.displayKind] stays a raw
+ * string because the backend can add kinds; these names keep the ones the app
+ * interprets in one place.
+ */
+internal object DisplayKind {
+    const val STEER = "steer"
+    const val CLARIFY_RESPONSE = "clarify_response"
+    const val LOCAL_FEEDBACK = "local_feedback"
+    const val MODEL_SWITCH = "model_switch"
+    const val PERSONALITY_SWITCH = "personality_switch"
+    const val AUTO_CONTINUE = "auto_continue"
+    const val ASYNC_DELEGATION_COMPLETE = "async_delegation_complete"
+    const val SKILL_INVOCATION = "skill_invocation"
+    const val INTERNAL_NOTIFICATION = "internal_notification"
+    const val MAX_ITERATIONS_REACHED = "max_iterations_reached"
+
+    /** Kinds that tag real user or local rows rather than timeline markers. */
+    val nonMarkerKinds = setOf(STEER, CLARIFY_RESPONSE, LOCAL_FEEDBACK)
+}
+
+/**
+ * The single owner of the REST transcript row-id format (#1336):
+ * `rest-<sessionId>-<rowKey>`, where rowKey is the server row id under newest-anchored
+ * paging or an absolute position under legacy paging.
+ */
+internal object RestMessageId {
+    private const val PREFIX = "rest-"
+
+    fun of(
+        sessionId: String,
+        rowKey: Any,
+    ): String = "$PREFIX$sessionId-$rowKey"
+
+    fun sessionPrefix(sessionId: String): String = "$PREFIX$sessionId-"
+
+    fun isRest(id: String): Boolean = id.startsWith(PREFIX)
+}

app/src/main/java/com/m57/hermescontrol/ui/chat/ChatServerMessageMapper.kt

@@ -51,9 +51,9 @@ internal fun mapServerMessages(
 
     fun restIdAt(index: Int): String =
         if (stableRowIds) {
-            "rest-$sessionId-${requireNotNull(messages[index].id) { "Latest transcript row has no stable id" }}"
+            RestMessageId.of(sessionId, requireNotNull(messages[index].id) { "Latest transcript row has no stable id" })
         } else {
-            "rest-$sessionId-${offset + index}"
+            RestMessageId.of(sessionId, offset + index)
         }
 
     val wsCompletionIdByRestIndex = mutableMapOf<Int, String>()
@@ -278,7 +278,7 @@ internal fun mapServerMessages(
     }
 
     // REST echoes must not reserve a match before the richer WS copy of that tool.
-    val liveTools = liveMessages.filter { it.role == MessageRole.TOOL && !it.id.startsWith("rest-") }
+    val liveTools = liveMessages.filter { it.role == MessageRole.TOOL && !RestMessageId.isRest(it.id) }
     val mappedTools = mapped.filter { it.role == MessageRole.TOOL }
     val matches = matchTranscriptMessages(mappedTools, liveTools)
     val toolsById = mappedTools.indices.associate { index -> mappedTools[index].id to matches[index] }

app/src/main/java/com/m57/hermescontrol/ui/chat/ChatTranscriptMerge.kt

@@ -174,12 +174,12 @@ internal class TranscriptComparison(
         // 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")
+            (a.displayKind == DisplayKind.LOCAL_FEEDBACK || b.displayKind == DisplayKind.LOCAL_FEEDBACK)
         ) {
             return false
         }
         if (a.role == MessageRole.USER &&
-            (a.displayKind == "clarify_response" || b.displayKind == "clarify_response")
+            (a.displayKind == DisplayKind.CLARIFY_RESPONSE || b.displayKind == DisplayKind.CLARIFY_RESPONSE)
         ) {
             return false
         }
@@ -323,8 +323,8 @@ internal fun dedupeCachedMessages(
     confirmedOnly: Boolean = false,
 ): List<ChatMessage> {
     val unique = messages.dedupeById()
-    val rest = unique.filter { it.id.startsWith("rest-") }
+    val rest = unique.filter { RestMessageId.isRest(it.id) }
-    val live = unique.filterNot { it.id.startsWith("rest-") }
+    val live = unique.filterNot { RestMessageId.isRest(it.id) }
     if (rest.isEmpty() || live.isEmpty()) return unique
     val matches =
         matchTranscriptMessages(rest, live).mapIndexed { index, match ->
@@ -405,7 +405,7 @@ internal fun mergeCachedTranscriptPage(
                             )
                         }
 
-                        match.id.startsWith("rest-") && !message.id.startsWith("rest-") -> {
+                        RestMessageId.isRest(match.id) && !RestMessageId.isRest(message.id) -> {
                             message
                         }
 
@@ -463,7 +463,7 @@ internal fun mergeTranscriptWithLive(
                     )
                 }
 
-                preserveLiveIds && match != null && !match.id.startsWith("rest-") -> {
+                preserveLiveIds && match != null && !RestMessageId.isRest(match.id) -> {
                     val mergedContent =
                         if (match.role == MessageRole.ASSISTANT &&
                             ChatVerifierFooter.split(match.content) != null &&
@@ -586,8 +586,8 @@ private fun List<ChatMessage>.inTranscriptOrder(
 
 internal fun ChatMessage.isPermanentlyLocal(): Boolean =
     role == MessageRole.SYSTEM ||
-        (role == MessageRole.USER && (content.startsWith("/") || displayKind == "clarify_response")) ||
+        (role == MessageRole.USER && (content.startsWith("/") || displayKind == DisplayKind.CLARIFY_RESPONSE)) ||
-        (role == MessageRole.ASSISTANT && displayKind == "local_feedback")
+        (role == MessageRole.ASSISTANT && displayKind == DisplayKind.LOCAL_FEEDBACK)
 
 internal fun ChatMessage.isSessionStartMarker(): Boolean =
     role == MessageRole.SYSTEM && (content == "Session created" || content == "Session branched")
@@ -635,6 +635,6 @@ internal fun mergeIncrementalTranscriptPage(
 internal fun serverMessageIndex(
     id: String,
     sessionId: String,
-): Int? = id.removePrefix("rest-$sessionId-").takeIf { it != id }?.toIntOrNull()
+): Int? = id.removePrefix(RestMessageId.sessionPrefix(sessionId)).takeIf { it != id }?.toIntOrNull()
 
 internal fun List<ChatMessage>.dedupeById(): List<ChatMessage> = associateBy { it.id }.values.toList()

app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt

@@ -4136,7 +4136,7 @@ class ChatViewModel(
             ChatMessage(
                 role = MessageRole.ASSISTANT,
                 content = text,
-                displayKind = "local_feedback",
+                displayKind = DisplayKind.LOCAL_FEEDBACK,
             )
         _uiState.update { it.copy(messages = it.messages + msg) }
 
@@ -4748,7 +4748,7 @@ class ChatViewModel(
                                     )
                                 }
                             if (!valid()) return@launch
-                            val targetId = "rest-$sessionId-$rowId"
+                            val targetId = RestMessageId.of(sessionId, rowId)
                             if (page.none { it.id == targetId || it.canonicalRestId == targetId }) {
                                 _timelineState.update {
                                     it.copy(windowErrorMessage = "The selected prompt is no longer available.")
@@ -5106,7 +5106,7 @@ class ChatViewModel(
         // A mapped page can reuse a live WS message. Never overwrite its newer persisted
         // version with the snapshot used for mapping; WS owns persistence of those IDs.
         val pageIds = page.mapNotNull { it.canonicalRestId }.toSet()
-        val aliases = _uiState.value.messages.filter { it.restId in pageIds && !it.id.startsWith("rest-") }
+        val aliases = _uiState.value.messages.filter { it.restId in pageIds && !RestMessageId.isRest(it.id) }
         withContext(historyDispatcher) {
             repo.persistMessages(
                 page.mapNotNull { message -> message.canonicalRestId?.let { message.copy(id = it, restId = null) } },

app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedTurns.kt

@@ -1,6 +1,7 @@
 package com.m57.hermescontrol.ui.chat.fullbleed
 
 import com.m57.hermescontrol.ui.chat.ChatMessage
+import com.m57.hermescontrol.ui.chat.DisplayKind
 import com.m57.hermescontrol.ui.chat.MessageRole
 import com.m57.hermescontrol.ui.chat.SearchMatch
 import com.m57.hermescontrol.ui.chat.SearchTarget
@@ -59,11 +60,7 @@ internal fun ChatMessage.isSyntheticSystemRow(): Boolean =
         displayKind == null &&
         content.startsWith(MAX_ITERATIONS_SYSTEM_MARKER)
 
-internal fun ChatMessage.isTimelineMarker(): Boolean =
+internal fun ChatMessage.isTimelineMarker(): Boolean = displayKind != null && displayKind !in DisplayKind.nonMarkerKinds
-    displayKind != null &&
-        displayKind != "steer" &&
-        displayKind != "clarify_response" &&
-        displayKind != "local_feedback"
 
 /**
  * Split a flat message list into turns for the full-bleed renderer.
@@ -99,7 +96,7 @@ fun groupIntoTurns(messages: List<ChatMessage>): List<ChatTurn> {
 
             message.isSyntheticSystemRow() -> {
                 agentEntries +=
-                    AgentEntry.SystemEvent(message.copy(displayKind = "max_iterations_reached"))
+                    AgentEntry.SystemEvent(message.copy(displayKind = DisplayKind.MAX_ITERATIONS_REACHED))
             }
 
             message.role == MessageRole.USER -> {

app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/TimelineMarkerChip.kt

@@ -15,6 +15,7 @@ import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.unit.dp
 import com.m57.hermescontrol.R
 import com.m57.hermescontrol.ui.chat.ChatMessage
+import com.m57.hermescontrol.ui.chat.DisplayKind
 
 /**
  * Friendly label resource for a `display_kind` timeline marker (issue #904).
@@ -24,13 +25,13 @@ import com.m57.hermescontrol.ui.chat.ChatMessage
  */
 internal fun timelineMarkerLabelRes(kind: String?): Int? =
     when (kind) {
-        "model_switch" -> R.string.timeline_marker_model_switch
+        DisplayKind.MODEL_SWITCH -> R.string.timeline_marker_model_switch
-        "personality_switch" -> R.string.timeline_marker_personality_switch
+        DisplayKind.PERSONALITY_SWITCH -> R.string.timeline_marker_personality_switch
-        "auto_continue" -> R.string.timeline_marker_auto_continue
+        DisplayKind.AUTO_CONTINUE -> R.string.timeline_marker_auto_continue
-        "async_delegation_complete" -> R.string.timeline_marker_delegation_complete
+        DisplayKind.ASYNC_DELEGATION_COMPLETE -> R.string.timeline_marker_delegation_complete
-        "skill_invocation" -> R.string.timeline_marker_skill_invocation
+        DisplayKind.SKILL_INVOCATION -> R.string.timeline_marker_skill_invocation
-        "internal_notification" -> R.string.timeline_marker_internal_notification
+        DisplayKind.INTERNAL_NOTIFICATION -> R.string.timeline_marker_internal_notification
-        "max_iterations_reached" -> R.string.timeline_marker_max_iterations
+        DisplayKind.MAX_ITERATIONS_REACHED -> R.string.timeline_marker_max_iterations
         else -> null
     }
 
@@ -63,7 +64,7 @@ internal fun TimelineMarkerChip(
 ) {
     val labelRes = timelineMarkerLabelRes(message.displayKind) ?: return
     val text =
-        if (message.displayKind == "model_switch") {
+        if (message.displayKind == DisplayKind.MODEL_SWITCH) {
             val model = markerModelFromContent(message.content)
             if (model != null) {
                 stringResource(R.string.timeline_marker_model_switch_to, model)

app/src/test/java/com/m57/hermescontrol/ui/chat/ChatMessageIdentityTest.kt

@@ -0,0 +1,34 @@
+package com.m57.hermescontrol.ui.chat
+
+import com.m57.hermescontrol.data.local.canonicalMessageOrder
+import com.m57.hermescontrol.ui.chat.fullbleed.isTimelineMarker
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+/** Issue #1336: one owner for the REST row-id format and the known display kinds. */
+class ChatMessageIdentityTest {
+    @Test
+    fun restIdsRoundTripThroughTheSingleFormatOwner() {
+        val id = RestMessageId.of("s-1", 42)
+
+        assertEquals("rest-s-1-42", id)
+        assertTrue(RestMessageId.isRest(id))
+        assertFalse(RestMessageId.isRest("ws-42"))
+        assertEquals(42L, canonicalMessageOrder(id, "s-1"))
+        assertNull(canonicalMessageOrder(RestMessageId.of("other", 42), "s-1"))
+        assertEquals(id, ChatMessage(id = id, role = MessageRole.USER, content = "").canonicalRestId)
+    }
+
+    @Test
+    fun onlyUnknownOrMarkerKindsAreTimelineMarkers() {
+        fun kind(value: String?) = ChatMessage(role = MessageRole.USER, content = "x", displayKind = value)
+
+        DisplayKind.nonMarkerKinds.forEach { assertFalse(it, kind(it).isTimelineMarker()) }
+        assertTrue(kind(DisplayKind.MODEL_SWITCH).isTimelineMarker())
+        assertTrue(kind("future_backend_kind").isTimelineMarker())
+        assertFalse(kind(null).isTimelineMarker())
+    }
+}