diff --git a/app/src/main/java/com/m57/hermescontrol/data/model/SessionMessagesResponse.kt b/app/src/main/java/com/m57/hermescontrol/data/model/SessionMessagesResponse.kt index 9ea2521a..a5a5e1a1 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/model/SessionMessagesResponse.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/model/SessionMessagesResponse.kt @@ -1,6 +1,7 @@ package com.m57.hermescontrol.data.model import kotlinx.serialization.Serializable +import kotlinx.serialization.json.JsonArray import kotlinx.serialization.json.JsonElement import kotlinx.serialization.json.JsonPrimitive import kotlinx.serialization.json.doubleOrNull @@ -59,6 +60,17 @@ data class SessionMessage( val display_metadata: JsonElement? = null, /** Token count recorded by the backend. */ val token_count: JsonElement? = null, + /** + * Public interim commentary projected by the backend for assistant rows + * (agent/history_commentary.py, issue #1284). Rendered as assistant text + * ahead of the reply. Absent on older gateways. + */ + val display_commentary: JsonElement? = null, + /** + * [reasoning] with the flattened commentary removed. When present (even + * empty) it wins over the raw reasoning so commentary is not shown twice. + */ + val display_reasoning: JsonElement? = null, ) { val timestampText: String? get() = (timestamp as? JsonPrimitive)?.content @@ -107,6 +119,41 @@ data class SessionMessage( else -> r.toString() } + /** True when the backend projected reasoning; it is then authoritative, even if empty. */ + val hasDisplayReasoning: Boolean + get() = (display_reasoning as? JsonPrimitive)?.isString == true + + /** [display_reasoning] when projected, else raw reasoning (older gateways). */ + val displayReasoningText: String + get() = (display_reasoning as? JsonPrimitive)?.takeIf { it.isString }?.content ?: reasoningText + + /** Nonblank public commentary items, in order. */ + val displayCommentary: List + get() = + (display_commentary as? JsonArray) + ?.mapNotNull { (it as? JsonPrimitive)?.takeIf { p -> p.isString }?.content?.trim() } + ?.filter { it.isNotEmpty() } + .orEmpty() + + /** + * Visible assistant text: interim commentary, then the reply. Commentary that + * merely repeats the reply (some providers persist it as content) renders once. + */ + val visibleText: String + get() { + val reply = displayContentText ?: contentText + val commentary = displayCommentary + if (commentary.isEmpty()) return reply + val joined = commentary.joinToString("\n\n") + if (reply.isBlank()) return joined + + fun normalized(value: String) = value.replace(WHITESPACE, " ").trim() + if (normalized(joined) == normalized(reply)) return reply + return "$joined\n\n$reply" + } + val toolCallId: String get() = tool_call_id.orEmpty() } + +private val WHITESPACE = Regex("\\s+") diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatServerMessageMapper.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatServerMessageMapper.kt index 79a9972b..9bc3a25b 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatServerMessageMapper.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatServerMessageMapper.kt @@ -179,13 +179,18 @@ internal fun mapServerMessages( ?: existingById[restId]?.timestamp ?: System.currentTimeMillis() - val rawContent = msg.displayContentText ?: msg.contentText + val rawContent = msg.visibleText + // #1284: projected reasoning already excludes public commentary; it is authoritative. val rowReasoning = - msg.reasoningText.ifBlank { - if (role == MessageRole.ASSISTANT) { - reasoningSources[index]?.reasoningText.orEmpty() - } else { - existingById[restId]?.reasoningText.orEmpty() + if (msg.hasDisplayReasoning) { + msg.displayReasoningText + } else { + msg.reasoningText.ifBlank { + if (role == MessageRole.ASSISTANT) { + reasoningSources[index]?.reasoningText.orEmpty() + } else { + existingById[restId]?.reasoningText.orEmpty() + } } } diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatHistoryCommentaryTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatHistoryCommentaryTest.kt new file mode 100644 index 00000000..c51fcc5c --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatHistoryCommentaryTest.kt @@ -0,0 +1,118 @@ +package com.m57.hermescontrol.ui.chat + +import com.m57.hermescontrol.data.model.SessionMessage +import kotlinx.serialization.json.JsonArray +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonPrimitive +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Test + +/** Issue #1284: history rows carry display_commentary / display_reasoning projections. */ +class ChatHistoryCommentaryTest { + private fun row( + content: String, + reasoning: String? = null, + commentary: List? = null, + displayReasoning: String? = null, + id: Int = 7, + ) = SessionMessage( + id = id, + role = "assistant", + content = JsonPrimitive(content), + reasoning = reasoning?.let { JsonPrimitive(it) }, + display_commentary = commentary?.let { list -> JsonArray(list.map { JsonPrimitive(it) }) }, + display_reasoning = displayReasoning?.let { JsonPrimitive(it) as JsonElement }, + ) + + private fun map(vararg rows: SessionMessage) = mapServerMessages("s", rows.toList(), 0, true, emptyList()) + + private fun countOf( + haystack: String, + needle: String, + ) = haystack.windowed(needle.length).count { it == needle } + + @Test + fun commentaryRendersOnceBeforeReplyAndLeavesReasoning() { + val commentary = "I will inspect the files." + val message = + map( + row( + content = "Done: two files changed.", + reasoning = "Private plan.\n\n$commentary", + commentary = listOf(commentary), + displayReasoning = "Private plan.", + ), + ).single() + + assertEquals("$commentary\n\nDone: two files changed.", message.content) + assertEquals("Private plan.", message.reasoningText) + assertEquals(1, countOf(message.content + message.reasoningText, commentary)) + } + + @Test + fun emptyDisplayReasoningWinsOverRawReasoning() { + val commentary = "Checking." + val message = + map(row(content = "", reasoning = commentary, commentary = listOf(commentary), displayReasoning = "")) + .single() + + assertEquals(commentary, message.content) + assertEquals("", message.reasoningText) + } + + @Test + fun commentaryEqualToReplyIsNotDuplicated() { + val text = "Looking at it now." + val message = map(row(content = text, commentary = listOf(text), displayReasoning = "")).single() + + assertEquals(text, message.content) + } + + @Test + fun disabledCommentaryKeepsItOutOfTranscriptAndReasoning() { + // Backend policy off: empty list, display_reasoning still strips the raw copy. + val message = + map( + row( + content = "Answer", + reasoning = "Plan.\nSecret interim", + commentary = emptyList(), + displayReasoning = "Plan.", + ), + ).single() + + assertEquals("Answer", message.content) + assertEquals("Plan.", message.reasoningText) + assertFalse(message.content.contains("Secret interim")) + } + + @Test + fun olderGatewayFallsBackToRawReasoning() { + val message = map(row(content = "Answer", reasoning = "Raw trace")).single() + + assertEquals("Answer", message.content) + assertEquals("Raw trace", message.reasoningText) + } + + @Test + fun liveSealedCommentaryBubbleIsCoveredByRestRowNotDuplicated() { + val commentary = "I will inspect the files." + val liveOrphan = ChatMessage(id = "live-orphan", role = MessageRole.ASSISTANT, content = commentary) + val rest = + map(row(content = "", reasoning = commentary, commentary = listOf(commentary), displayReasoning = "")) + + val merged = mergeTranscriptWithLive(rest, listOf(liveOrphan), preserveLiveIds = true) + + assertEquals(1, merged.count { it.content.contains(commentary) }) + assertEquals("", merged.single().reasoningText) + } + + @Test + fun multipleCommentaryItemsKeepOrderAndSkipBlanks() { + val message = + map(row(content = "Final", commentary = listOf("First.", " ", "Second."), displayReasoning = "")).single() + + assertEquals("First.\n\nSecond.\n\nFinal", message.content) + } +}