This commit is contained in:
3 files changed
+176
-6
No files matched your search
@@ -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<String>
|
||||
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+")
|
||||
@@ -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()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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<String>? = 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)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user