refactor(chat): pure server message mapper (#1363)
This commit is contained in:
5 files changed
+179
-32
No files matched your search
@@ -251,7 +251,15 @@ class ChatReadNotificationTest {
|
||||
SessionMessage(id = 10, role = "assistant", content = JsonPrimitive("Done")),
|
||||
SessionMessage(id = 52, role = "assistant", content = JsonPrimitive("Done")),
|
||||
)
|
||||
val messages = mapServerMessages(sessionId, history, 0, true, emptyList(), context = context)
|
||||
val messages =
|
||||
mapServerMessages(
|
||||
sessionId,
|
||||
history,
|
||||
0,
|
||||
true,
|
||||
emptyList(),
|
||||
activeReplyTarget = ReplyNotificationTracker.getActiveTarget(context),
|
||||
)
|
||||
assertEquals("Older duplicate must stay unmapped", null, messages[0].completionId)
|
||||
assertEquals("Exact row must receive the completion id", "comp-exact-row", messages[1].completionId)
|
||||
|
||||
|
||||
@@ -1,20 +1,24 @@
|
||||
package com.m57.hermescontrol.ui.chat
|
||||
|
||||
import com.m57.hermescontrol.data.local.AuthManager
|
||||
import com.m57.hermescontrol.data.model.Attachment
|
||||
import com.m57.hermescontrol.data.model.AttachmentSource
|
||||
import com.m57.hermescontrol.data.model.SessionMessage
|
||||
import com.m57.hermescontrol.data.remote.GatewayFileClient
|
||||
import com.m57.hermescontrol.notification.ReplyNotificationTracker
|
||||
import com.m57.hermescontrol.notification.ReplyNotificationTarget
|
||||
import com.m57.hermescontrol.ui.chat.tool.ToolJson
|
||||
import kotlinx.serialization.json.JsonObject
|
||||
|
||||
/**
|
||||
* Maps REST transcript rows ([SessionMessage]) into UI [ChatMessage]s.
|
||||
*
|
||||
* Reads the supplied transcript snapshot and recovers notification identity through
|
||||
* ReplyNotificationTracker's synchronized lookup. Safe on the history dispatcher;
|
||||
* it never mutates ViewModel state or dismisses a notification.
|
||||
* Pure: every input is plain data. Callers resolve the active reply-notification
|
||||
* target and the gateway media URL builder, so this never touches Android,
|
||||
* AuthManager, or the notification tracker (#1337).
|
||||
*
|
||||
* @param activeReplyTarget the reply notification currently posted, if any; its
|
||||
* durable server row id reserves that row's completion identity.
|
||||
* @param mediaUrl builds an authenticated gateway URL for a `MEDIA:` path, or null
|
||||
* when no gateway is configured.
|
||||
* @param nowMs fallback timestamp for rows without one.
|
||||
*/
|
||||
internal fun mapServerMessages(
|
||||
sessionId: String,
|
||||
@@ -24,7 +28,9 @@ internal fun mapServerMessages(
|
||||
liveMessages: List<ChatMessage>,
|
||||
isPagingOlder: Boolean = false,
|
||||
stableRowIds: Boolean = latestPaging,
|
||||
context: android.content.Context? = null,
|
||||
activeReplyTarget: ReplyNotificationTarget? = null,
|
||||
mediaUrl: (path: String) -> String? = { null },
|
||||
nowMs: Long = System.currentTimeMillis(),
|
||||
): List<ChatMessage> {
|
||||
val existingById = liveMessages.associateBy { it.canonicalRestId ?: it.id }
|
||||
val liveByExactId =
|
||||
@@ -55,7 +61,7 @@ internal fun mapServerMessages(
|
||||
if (!isPagingOlder) {
|
||||
// #129: reserve durable identity before matching any repeated live prose.
|
||||
// REST-only recovery remains fail-closed when the target row is absent.
|
||||
val activeTarget = ReplyNotificationTracker.getActiveTarget(context)?.takeIf { it.sessionId == sessionId }
|
||||
val activeTarget = activeReplyTarget?.takeIf { it.sessionId == sessionId }
|
||||
val durableTarget = activeTarget?.takeIf { it.serverMessageId != null }
|
||||
if (durableTarget != null) {
|
||||
val exactIndex =
|
||||
@@ -179,7 +185,7 @@ internal fun mapServerMessages(
|
||||
?.times(1000)
|
||||
?.toLong()
|
||||
?: existingById[restId]?.timestamp
|
||||
?: System.currentTimeMillis()
|
||||
?: nowMs
|
||||
|
||||
val rawContent =
|
||||
if (role == MessageRole.TOOL && msg.display_metadata != null) {
|
||||
@@ -229,18 +235,11 @@ internal fun mapServerMessages(
|
||||
if (role == MessageRole.ASSISTANT && rawContent.contains("MEDIA:")) {
|
||||
val items = HostMediaExtractor.extract(rawContent)
|
||||
if (items.isNotEmpty()) {
|
||||
val baseUrl = AuthManager.getBaseUrl()
|
||||
val token = AuthManager.getToken().orEmpty()
|
||||
finalContent = HostMediaExtractor.strip(rawContent)
|
||||
attachments =
|
||||
items
|
||||
.mapNotNull { item ->
|
||||
val url =
|
||||
GatewayFileClient.buildMediaUrl(
|
||||
baseUrl,
|
||||
token,
|
||||
item.path,
|
||||
) ?: return@mapNotNull null
|
||||
val url = mediaUrl(item.path) ?: return@mapNotNull null
|
||||
Attachment(
|
||||
uri = url,
|
||||
name = mediaNameFromPath(item.path),
|
||||
|
||||
@@ -21,6 +21,7 @@ import com.m57.hermescontrol.data.model.parseContextBreakdown
|
||||
import com.m57.hermescontrol.data.model.parseUsageSnapshot
|
||||
import com.m57.hermescontrol.data.model.reasoningSupport
|
||||
import com.m57.hermescontrol.data.remote.ApiClient
|
||||
import com.m57.hermescontrol.data.remote.GatewayFileClient
|
||||
import com.m57.hermescontrol.data.remote.NetworkResult
|
||||
import com.m57.hermescontrol.data.remote.OkHttpProvider
|
||||
import com.m57.hermescontrol.data.remote.safeApiCall
|
||||
@@ -38,6 +39,7 @@ import com.m57.hermescontrol.data.ws.WsEvent
|
||||
import com.m57.hermescontrol.data.ws.WsMethods
|
||||
import com.m57.hermescontrol.data.ws.toAny
|
||||
import com.m57.hermescontrol.data.ws.toJsonElement
|
||||
import com.m57.hermescontrol.notification.ReplyNotificationTracker
|
||||
import com.m57.hermescontrol.notification.captureTurnBoundary
|
||||
import com.m57.hermescontrol.notification.correlationScopeId
|
||||
import com.m57.hermescontrol.ui.chat.fullbleed.TranscriptUiState
|
||||
@@ -2249,6 +2251,10 @@ class ChatViewModel(
|
||||
|
||||
// ── Send message ─────────────────────────────────────────────────────
|
||||
|
||||
/** Authenticated gateway URL for a host `MEDIA:` path; kept out of the pure mapper (#1337). */
|
||||
private fun gatewayMediaUrl(path: String): String? =
|
||||
GatewayFileClient.buildMediaUrl(AuthManager.getBaseUrl(), AuthManager.getToken().orEmpty(), path)
|
||||
|
||||
private fun sendScope(): String =
|
||||
listOf(
|
||||
AuthManager.getBaseUrl(),
|
||||
@@ -4242,7 +4248,8 @@ class ChatViewModel(
|
||||
offset = 0,
|
||||
latestPaging = latestPaging,
|
||||
liveMessages = emptyList(),
|
||||
context = getApplication(),
|
||||
activeReplyTarget = ReplyNotificationTracker.getActiveTarget(getApplication()),
|
||||
mediaUrl = ::gatewayMediaUrl,
|
||||
)
|
||||
}
|
||||
_uiState.update { it.copy(messages = replacementMessages) }
|
||||
@@ -4736,7 +4743,8 @@ class ChatViewModel(
|
||||
liveMessages = _uiState.value.messages,
|
||||
isPagingOlder = true,
|
||||
stableRowIds = true,
|
||||
context = getApplication(),
|
||||
activeReplyTarget = ReplyNotificationTracker.getActiveTarget(getApplication()),
|
||||
mediaUrl = ::gatewayMediaUrl,
|
||||
)
|
||||
}
|
||||
if (!valid()) return@launch
|
||||
@@ -5046,7 +5054,8 @@ class ChatViewModel(
|
||||
serverOffset,
|
||||
useLatest,
|
||||
current,
|
||||
context = getApplication(),
|
||||
activeReplyTarget = ReplyNotificationTracker.getActiveTarget(getApplication()),
|
||||
mediaUrl = ::gatewayMediaUrl,
|
||||
)
|
||||
} ?: return@launch
|
||||
persistHistoryPage(page, sessionId)
|
||||
@@ -5503,7 +5512,8 @@ class ChatViewModel(
|
||||
useLatest,
|
||||
current,
|
||||
isPagingOlder = true,
|
||||
context = getApplication(),
|
||||
activeReplyTarget = ReplyNotificationTracker.getActiveTarget(getApplication()),
|
||||
mediaUrl = ::gatewayMediaUrl,
|
||||
)
|
||||
} ?: return@launch
|
||||
persistHistoryPage(page, sessionId)
|
||||
@@ -5584,7 +5594,8 @@ class ChatViewModel(
|
||||
current,
|
||||
// Sync fetches recent replies, so confirm live completion identities.
|
||||
isPagingOlder = false,
|
||||
context = getApplication(),
|
||||
activeReplyTarget = ReplyNotificationTracker.getActiveTarget(getApplication()),
|
||||
mediaUrl = ::gatewayMediaUrl,
|
||||
)
|
||||
} ?: return@launch
|
||||
persistHistoryPage(page, sessionId)
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
package com.m57.hermescontrol.ui.chat
|
||||
|
||||
import com.m57.hermescontrol.data.model.SessionMessage
|
||||
import com.m57.hermescontrol.notification.ReplyNotificationTarget
|
||||
import kotlinx.serialization.json.JsonPrimitive
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Test
|
||||
|
||||
/** Issue #1337: the REST mapper runs on plain inputs, with no Android, AuthManager, or tracker state. */
|
||||
class ChatServerMessageMapperPurityTest {
|
||||
private fun assistant(
|
||||
id: Int,
|
||||
text: String,
|
||||
) = SessionMessage(id = id, role = "assistant", content = JsonPrimitive(text))
|
||||
|
||||
@Test
|
||||
fun mediaAttachmentsUseTheInjectedUrlBuilder() {
|
||||
val mapped =
|
||||
mapServerMessages(
|
||||
"s",
|
||||
listOf(assistant(1, "Chart MEDIA:/opt/hermes/chart.png")),
|
||||
0,
|
||||
true,
|
||||
emptyList(),
|
||||
mediaUrl = { path -> "gw://$path" },
|
||||
).single()
|
||||
|
||||
assertEquals("Chart", mapped.content.trim())
|
||||
assertEquals(listOf("gw:///opt/hermes/chart.png"), mapped.attachments?.map { it.uri })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun mediaWithoutAGatewayStripsTheTokenAndAddsNoAttachment() {
|
||||
val mapped =
|
||||
mapServerMessages("s", listOf(assistant(1, "Chart MEDIA:/opt/hermes/chart.png")), 0, true, emptyList())
|
||||
.single()
|
||||
|
||||
assertEquals("Chart", mapped.content.trim())
|
||||
assertNull(mapped.attachments)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun activeReplyTargetIsAPlainInputScopedToItsSession() {
|
||||
val rows = listOf(assistant(100, "Done"), assistant(200, "Done"))
|
||||
val target = ReplyNotificationTarget("default", "s", "comp", 1L, serverMessageId = 100)
|
||||
|
||||
val bound = mapServerMessages("s", rows, 0, true, emptyList(), activeReplyTarget = target)
|
||||
val otherSession =
|
||||
mapServerMessages("s", rows, 0, true, emptyList(), activeReplyTarget = target.copy(sessionId = "x"))
|
||||
|
||||
assertEquals(listOf("comp", null), bound.map { it.completionId })
|
||||
assertEquals(listOf(null, null), otherSession.map { it.completionId })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun rowsWithoutTimestampsUseTheInjectedClock() {
|
||||
val mapped = mapServerMessages("s", listOf(assistant(1, "Hi")), 0, true, emptyList(), nowMs = 42L)
|
||||
|
||||
assertEquals(42L, mapped.single().timestamp)
|
||||
}
|
||||
}
|
||||
+76
-9
@@ -169,11 +169,29 @@ class ReadNotificationReviewRegressionTest {
|
||||
SessionMessage(id = 200, role = "assistant", content = JsonPrimitive("Done")),
|
||||
)
|
||||
for (latest in listOf(true, false)) {
|
||||
val mapped = mapServerMessages("session", rows, 10, latest, listOf(live), context = context)
|
||||
val mapped =
|
||||
mapServerMessages(
|
||||
"session",
|
||||
rows,
|
||||
10,
|
||||
latest,
|
||||
listOf(live),
|
||||
activeReplyTarget = ReplyNotificationTracker.getActiveTarget(context),
|
||||
)
|
||||
assertEquals(listOf("comp", null), mapped.map { it.completionId })
|
||||
val merged = mergeTranscriptWithLive(mapped, listOf(live), preserveLiveIds = true)
|
||||
assertEquals(if (latest) "rest-session-100" else "rest-session-10", merged.single { it.id == "ws" }.restId)
|
||||
assertNull(mapServerMessages("session", rows.takeLast(1), 11, latest, listOf(live)).single().completionId)
|
||||
// Target row absent from this page: the reserved completion must fail closed, not rebind by text.
|
||||
val targetRowAbsent =
|
||||
mapServerMessages(
|
||||
"session",
|
||||
rows.takeLast(1),
|
||||
11,
|
||||
latest,
|
||||
listOf(live),
|
||||
activeReplyTarget = ReplyNotificationTracker.getActiveTarget(context),
|
||||
)
|
||||
assertNull(targetRowAbsent.single().completionId)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -214,6 +232,7 @@ class ReadNotificationReviewRegressionTest {
|
||||
0,
|
||||
true,
|
||||
current,
|
||||
activeReplyTarget = ReplyNotificationTracker.getActiveTarget(),
|
||||
)
|
||||
val merged = mergeTranscriptWithLive(mapped, current, preserveLiveIds = true).single()
|
||||
assertEquals("ws", merged.id)
|
||||
@@ -256,7 +275,7 @@ class ReadNotificationReviewRegressionTest {
|
||||
latestPaging = true,
|
||||
liveMessages = current,
|
||||
isPagingOlder = false,
|
||||
context = context,
|
||||
activeReplyTarget = ReplyNotificationTracker.getActiveTarget(context),
|
||||
)
|
||||
val merged = mergeTranscriptWithLive(mapped, current, preserveLiveIds = true)
|
||||
|
||||
@@ -685,7 +704,15 @@ class ReadNotificationReviewRegressionTest {
|
||||
)
|
||||
|
||||
val mapped =
|
||||
mapServerMessages("session", history, 0, true, emptyList(), isPagingOlder = false, context = context)
|
||||
mapServerMessages(
|
||||
"session",
|
||||
history,
|
||||
0,
|
||||
true,
|
||||
emptyList(),
|
||||
isPagingOlder = false,
|
||||
activeReplyTarget = ReplyNotificationTracker.getActiveTarget(context),
|
||||
)
|
||||
assertEquals("Older message must have null completionId", null, mapped[0].completionId)
|
||||
assertEquals("Newest message must receive target completionId", "comp-newest-done", mapped[1].completionId)
|
||||
|
||||
@@ -747,7 +774,15 @@ class ReadNotificationReviewRegressionTest {
|
||||
)
|
||||
|
||||
val mapped =
|
||||
mapServerMessages("session", history, 0, true, emptyList(), isPagingOlder = false, context = context)
|
||||
mapServerMessages(
|
||||
"session",
|
||||
history,
|
||||
0,
|
||||
true,
|
||||
emptyList(),
|
||||
isPagingOlder = false,
|
||||
activeReplyTarget = ReplyNotificationTracker.getActiveTarget(context),
|
||||
)
|
||||
assertEquals("comp-long-2", mapped[0].completionId)
|
||||
assertEquals(null, mapped[1].completionId)
|
||||
|
||||
@@ -796,7 +831,15 @@ class ReadNotificationReviewRegressionTest {
|
||||
)
|
||||
|
||||
val mapped =
|
||||
mapServerMessages("session", history, 0, true, emptyList(), isPagingOlder = false, context = context)
|
||||
mapServerMessages(
|
||||
"session",
|
||||
history,
|
||||
0,
|
||||
true,
|
||||
emptyList(),
|
||||
isPagingOlder = false,
|
||||
activeReplyTarget = ReplyNotificationTracker.getActiveTarget(context),
|
||||
)
|
||||
val row = mapped.single()
|
||||
assertEquals("comp-media-cold", row.completionId)
|
||||
assertEquals("Here is the image", row.content)
|
||||
@@ -839,7 +882,15 @@ class ReadNotificationReviewRegressionTest {
|
||||
)
|
||||
|
||||
val mapped =
|
||||
mapServerMessages("session", history, 0, true, emptyList(), isPagingOlder = false, context = context)
|
||||
mapServerMessages(
|
||||
"session",
|
||||
history,
|
||||
0,
|
||||
true,
|
||||
emptyList(),
|
||||
isPagingOlder = false,
|
||||
activeReplyTarget = ReplyNotificationTracker.getActiveTarget(context),
|
||||
)
|
||||
val row = mapped.single()
|
||||
assertEquals("comp-media-only-cold", row.completionId)
|
||||
assertEquals("", row.content)
|
||||
@@ -887,7 +938,15 @@ class ReadNotificationReviewRegressionTest {
|
||||
)
|
||||
|
||||
val mapped =
|
||||
mapServerMessages("session", history, 0, true, emptyList(), isPagingOlder = false, context = context)
|
||||
mapServerMessages(
|
||||
"session",
|
||||
history,
|
||||
0,
|
||||
true,
|
||||
emptyList(),
|
||||
isPagingOlder = false,
|
||||
activeReplyTarget = ReplyNotificationTracker.getActiveTarget(context),
|
||||
)
|
||||
assertEquals("comp-media-dup-2", mapped[0].completionId)
|
||||
assertEquals(null, mapped[1].completionId)
|
||||
|
||||
@@ -955,7 +1014,15 @@ class ReadNotificationReviewRegressionTest {
|
||||
}
|
||||
|
||||
val mapped =
|
||||
mapServerMessages("session", history, 0, true, emptyList(), isPagingOlder = false, context = context)
|
||||
mapServerMessages(
|
||||
"session",
|
||||
history,
|
||||
0,
|
||||
true,
|
||||
emptyList(),
|
||||
isPagingOlder = false,
|
||||
activeReplyTarget = ReplyNotificationTracker.getActiveTarget(context),
|
||||
)
|
||||
assertEquals(listOf(null, null, "comp-turn"), mapped.map { it.completionId })
|
||||
|
||||
assertFalse(
|
||||
|
||||
Reference in new issue
Block a user