feat(chat): rewire tool bubbles onto the desktop engine (stage 3) (#765)
* feat(chat): rewire tool bubbles onto the desktop engine (stage 3)
parseToolOutput is now a thin adapter: it normalizes the payload shape
(args/result sub-objects, legacy top-level fallback, name resolution)
and delegates all per-tool formatting to ToolViewBuilder.
- ToolResultParser.kt: 1325 lines of bespoke formatters → 45-line adapter
- ParsedToolData deleted; ToolBubble renders ToolView directly:
collapsed = emoji + engine title (+countLabel) + subtitle line;
expanded = error → $ command / stdout / stderr / exit code →
DiffViewCard → search hits → detail → duration footer
- Gson-compat shims, formatTodoToolOutput, toIntOrNull and all
per-tool formatters died with the old parser (engine is idiomatic kotlinx)
- Kept mobile-only UI: progress preview, SecurityRiskChip, emoji
prefixes, raw-JSON toggle, DiffViewCard, copy
- ToolViewBuilder fixes found during the rewire: numberValue now accepts
numeric strings (TS Number() semantics — gateway ships exit_code as
string/float), memory title keeps its target suffix
ToolBubbleParsingTest rewritten as adapter-contract tests (14 cases);
per-tool behavior lives in ToolViewBuilderTest. Verified: ktlint +
full testDebugUnitTest.
* fix(chat): dedupe tool name in collapsed bubble summary
The header row already shows the tool name, but the collapsed summary
showed emoji + engine title — and for ~11 tools the engine title is just
the generic humanized name (fact_store -> "Fact Store", todo -> "Todo"),
so the bubble displayed the tool identity twice.
composeToolSummaryLines now drops the title when it equals the generic
humanized name and merges the subtitle into the emoji line instead
("🧠 Fact added (ID: 5)"), restoring the pre-engine one-line style.
Descriptive titles ("Searched \"cats\"", "Ran ls", "Read a.kt L10-14")
stay on line one with the subtitle on line two. A lone emoji with no
subtitle renders nothing.
ToolViewBuilder.genericTitleFor exposes the generic title for the
comparison. 9 new unit tests.
---------
Co-authored-by: hy4ri <hy4ri@users.noreply.github.com>
This commit is contained in:
M57andhy4ri authored and GitHub committed
1 parent
100a82dd4f
commit
982f8aec4e
6 files changed
+507
-1766
No files matched your search
@@ -1,20 +0,0 @@
|
||||
package com.m57.hermescontrol.ui.chat
|
||||
|
||||
/**
|
||||
* Cleanly parsed representation of a tool call, extracted from the
|
||||
* tool.complete JSON payload (which contains both args and result).
|
||||
*/
|
||||
data class ParsedToolData(
|
||||
val toolName: String = "",
|
||||
val isTerminal: Boolean = false,
|
||||
val stdout: String? = null,
|
||||
val exitCode: Int? = null,
|
||||
val error: String? = null,
|
||||
val summaryText: String? = null,
|
||||
val durationSec: Double? = null,
|
||||
val mainOutput: String? = null,
|
||||
val extraFields: Map<String, String> = emptyMap(),
|
||||
val isRunning: Boolean = false,
|
||||
val diffOutput: String? = null,
|
||||
val diffPath: String? = null,
|
||||
)
|
||||
@@ -60,132 +60,52 @@ import com.m57.hermescontrol.R
|
||||
import com.m57.hermescontrol.theme.HermesStatusColors
|
||||
import com.m57.hermescontrol.theme.LocalHermesStatusColors
|
||||
import com.m57.hermescontrol.ui.chat.components.DiffViewCard
|
||||
import com.m57.hermescontrol.ui.chat.tool.ToolView
|
||||
import com.m57.hermescontrol.ui.chat.tool.ToolViewBuilder
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.launch
|
||||
|
||||
@Composable
|
||||
private fun ExpandedToolContent(
|
||||
parsed: ParsedToolData,
|
||||
contentColor: Color,
|
||||
statusColors: HermesStatusColors,
|
||||
) {
|
||||
Column(verticalArrangement = Arrangement.spacedBy(4.dp)) {
|
||||
// Summary line at top of expanded view
|
||||
if (parsed.summaryText != null) {
|
||||
Text(
|
||||
text = parsed.summaryText,
|
||||
style =
|
||||
MaterialTheme.typography.bodySmall.copy(
|
||||
color = contentColor.copy(alpha = 0.7f),
|
||||
fontFamily = FontFamily.Monospace,
|
||||
fontSize = 11.sp,
|
||||
),
|
||||
)
|
||||
Spacer(modifier = Modifier.height(2.dp))
|
||||
}
|
||||
/**
|
||||
* Composes the collapsed summary lines for a tool card.
|
||||
*
|
||||
* The header row already shows the tool name, so when the engine's title is
|
||||
* just the generic humanized name ("Fact Store" for fact_store) it is
|
||||
* dropped and the subtitle merges into the emoji line instead — one
|
||||
* identity per bubble. Descriptive titles ("Searched \"cats\"", "Ran ls")
|
||||
* stay on line one with the subtitle on line two.
|
||||
*
|
||||
* Returns null when there is nothing to show (lone emoji, no subtitle).
|
||||
*/
|
||||
internal fun composeToolSummaryLines(
|
||||
view: ToolView,
|
||||
toolName: String?,
|
||||
emoji: String,
|
||||
): Pair<String, String?>? {
|
||||
val titleIsGeneric = view.title.equals(ToolViewBuilder.genericTitleFor(toolName), ignoreCase = true)
|
||||
val subtitle = view.subtitle.takeIf { it.isNotBlank() && it != view.title }
|
||||
|
||||
if (parsed.isTerminal) {
|
||||
// ── Terminal output ──
|
||||
parsed.stdout?.let {
|
||||
Text(
|
||||
text = it,
|
||||
style =
|
||||
MaterialTheme.typography.bodySmall.copy(
|
||||
color = contentColor.copy(alpha = 0.9f),
|
||||
fontFamily = FontFamily.Monospace,
|
||||
fontSize = 11.sp,
|
||||
),
|
||||
)
|
||||
}
|
||||
parsed.error?.let {
|
||||
Text(
|
||||
text = stringResource(R.string.chat_tool_execution_error, it),
|
||||
style =
|
||||
MaterialTheme.typography.bodySmall.copy(
|
||||
color = statusColors.error,
|
||||
fontWeight = FontWeight.Bold,
|
||||
fontFamily = FontFamily.Monospace,
|
||||
fontSize = 11.sp,
|
||||
),
|
||||
)
|
||||
}
|
||||
parsed.exitCode?.let { code ->
|
||||
if (code != 0) {
|
||||
Text(
|
||||
text = stringResource(R.string.chat_tool_exit_code, code),
|
||||
style =
|
||||
MaterialTheme.typography.labelSmall.copy(
|
||||
color = statusColors.error,
|
||||
fontWeight = FontWeight.Medium,
|
||||
),
|
||||
)
|
||||
val firstLine: String
|
||||
val secondLine: String?
|
||||
if (titleIsGeneric) {
|
||||
firstLine =
|
||||
buildString {
|
||||
append(emoji)
|
||||
if (subtitle != null) {
|
||||
append(" $subtitle")
|
||||
}
|
||||
view.countLabel?.let { append(" ($it)") }
|
||||
}
|
||||
|
||||
// Duration footer for terminal
|
||||
parsed.durationSec?.let { dur ->
|
||||
Text(
|
||||
text = "Duration: ${"%.1f".format(dur)}s",
|
||||
style =
|
||||
MaterialTheme.typography.labelSmall.copy(
|
||||
color = contentColor.copy(alpha = 0.5f),
|
||||
),
|
||||
)
|
||||
secondLine = null
|
||||
} else {
|
||||
firstLine =
|
||||
buildString {
|
||||
append("$emoji ${view.title}")
|
||||
view.countLabel?.let { append(" ($it)") }
|
||||
}
|
||||
} else {
|
||||
// ── Generic tool output / Diff view ──
|
||||
if (parsed.diffOutput != null) {
|
||||
DiffViewCard(
|
||||
diffText = parsed.diffOutput,
|
||||
filePath = parsed.diffPath,
|
||||
)
|
||||
} else {
|
||||
parsed.mainOutput?.let {
|
||||
Text(
|
||||
text = it,
|
||||
style =
|
||||
MaterialTheme.typography.bodySmall.copy(
|
||||
color = contentColor.copy(alpha = 0.9f),
|
||||
fontSize = 12.sp,
|
||||
),
|
||||
)
|
||||
}
|
||||
parsed.extraFields.forEach { (key, value) ->
|
||||
Row(
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
horizontalArrangement = Arrangement.spacedBy(4.dp),
|
||||
) {
|
||||
Text(
|
||||
text = "$key:",
|
||||
style =
|
||||
MaterialTheme.typography.labelSmall.copy(
|
||||
color = contentColor.copy(alpha = 0.6f),
|
||||
fontWeight = FontWeight.Bold,
|
||||
),
|
||||
)
|
||||
Text(
|
||||
text = value,
|
||||
style =
|
||||
MaterialTheme.typography.bodySmall.copy(
|
||||
color = contentColor.copy(alpha = 0.9f),
|
||||
fontSize = 11.sp,
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
// Duration footer
|
||||
parsed.durationSec?.let { dur ->
|
||||
Text(
|
||||
text = "Duration: ${"%.1f".format(dur)}s",
|
||||
style =
|
||||
MaterialTheme.typography.labelSmall.copy(
|
||||
color = contentColor.copy(alpha = 0.5f),
|
||||
),
|
||||
)
|
||||
}
|
||||
secondLine = subtitle
|
||||
}
|
||||
|
||||
return firstLine.takeIf { it.trim() != emoji }?.let { it to secondLine }
|
||||
}
|
||||
|
||||
@Composable
|
||||
@@ -199,7 +119,7 @@ internal fun ToolBubble(
|
||||
val contentColor = MaterialTheme.colorScheme.onSurfaceVariant
|
||||
val statusColors = LocalHermesStatusColors.current
|
||||
|
||||
val parsed =
|
||||
val view =
|
||||
remember(message.content, message.toolName, message.toolStatus) {
|
||||
parseToolOutput(message.content, message.toolName, message.toolStatus == ToolStatus.RUNNING)
|
||||
}
|
||||
@@ -260,20 +180,36 @@ internal fun ToolBubble(
|
||||
SecurityRiskChip(riskData, contentColor)
|
||||
}
|
||||
|
||||
// ── Summary line (always visible when collapsed) ──
|
||||
if (!expanded && parsed?.summaryText != null) {
|
||||
Text(
|
||||
text = parsed.summaryText,
|
||||
style =
|
||||
MaterialTheme.typography.bodySmall.copy(
|
||||
color = contentColor.copy(alpha = 0.7f),
|
||||
fontFamily = FontFamily.Monospace,
|
||||
fontSize = 11.sp,
|
||||
),
|
||||
maxLines = 1,
|
||||
overflow = TextOverflow.Ellipsis,
|
||||
modifier = Modifier.padding(top = 4.dp, start = 22.dp),
|
||||
)
|
||||
// ── Collapsed summary: emoji + title, then subtitle ──
|
||||
if (!expanded && view != null) {
|
||||
composeToolSummaryLines(view, message.toolName, config.iconEmoji)?.let { (firstLine, secondLine) ->
|
||||
Text(
|
||||
text = firstLine,
|
||||
style =
|
||||
MaterialTheme.typography.bodySmall.copy(
|
||||
color = contentColor.copy(alpha = 0.7f),
|
||||
fontFamily = FontFamily.Monospace,
|
||||
fontSize = 11.sp,
|
||||
),
|
||||
maxLines = 1,
|
||||
overflow = TextOverflow.Ellipsis,
|
||||
modifier = Modifier.padding(top = 4.dp, start = 22.dp),
|
||||
)
|
||||
if (secondLine != null) {
|
||||
Text(
|
||||
text = secondLine,
|
||||
style =
|
||||
MaterialTheme.typography.bodySmall.copy(
|
||||
color = contentColor.copy(alpha = 0.5f),
|
||||
fontFamily = FontFamily.Monospace,
|
||||
fontSize = 11.sp,
|
||||
),
|
||||
maxLines = 1,
|
||||
overflow = TextOverflow.Ellipsis,
|
||||
modifier = Modifier.padding(top = 1.dp, start = 22.dp),
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ── Expanded content ──
|
||||
@@ -318,11 +254,11 @@ internal fun ToolBubble(
|
||||
.testTag("chat_tool_show_parsed")
|
||||
.clickable(role = Role.Button) { showRawJson = false },
|
||||
)
|
||||
} else if (parsed != null) {
|
||||
} else if (view != null) {
|
||||
// Clean structured expanded view
|
||||
Box {
|
||||
SelectionContainer {
|
||||
ExpandedToolContent(parsed, contentColor, statusColors)
|
||||
ExpandedToolContent(view, contentColor, statusColors)
|
||||
}
|
||||
CopyButton(
|
||||
visible = showCopyButton,
|
||||
@@ -387,6 +323,173 @@ internal fun ToolBubble(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Renders the engine's [ToolView] — the expanded body of a tool card.
|
||||
*
|
||||
* Order: error line → terminal streams ($ command / stdout / stderr / exit
|
||||
* code) → file diff → search hits → plain detail → duration footer.
|
||||
*/
|
||||
@Composable
|
||||
private fun ExpandedToolContent(
|
||||
view: ToolView,
|
||||
contentColor: Color,
|
||||
statusColors: HermesStatusColors,
|
||||
) {
|
||||
Column(verticalArrangement = Arrangement.spacedBy(4.dp)) {
|
||||
val isTerminal =
|
||||
view.stdout != null ||
|
||||
view.stderr != null ||
|
||||
view.exitCode != null ||
|
||||
view.terminalCommand != null
|
||||
|
||||
// ── Error line ──
|
||||
view.error?.let {
|
||||
Text(
|
||||
text = stringResource(R.string.chat_tool_execution_error, it),
|
||||
style =
|
||||
MaterialTheme.typography.bodySmall.copy(
|
||||
color = statusColors.error,
|
||||
fontWeight = FontWeight.Bold,
|
||||
fontFamily = FontFamily.Monospace,
|
||||
fontSize = 11.sp,
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
// ── Terminal: $ command + streams + exit code ──
|
||||
if (isTerminal) {
|
||||
view.terminalCommand?.takeIf { it.isNotEmpty() }?.let { command ->
|
||||
Text(
|
||||
text = "$ $command",
|
||||
style =
|
||||
MaterialTheme.typography.bodySmall.copy(
|
||||
color = contentColor.copy(alpha = 0.7f),
|
||||
fontFamily = FontFamily.Monospace,
|
||||
fontSize = 11.sp,
|
||||
),
|
||||
)
|
||||
}
|
||||
view.stdout?.let {
|
||||
Text(
|
||||
text = it,
|
||||
style =
|
||||
MaterialTheme.typography.bodySmall.copy(
|
||||
color = contentColor.copy(alpha = 0.9f),
|
||||
fontFamily = FontFamily.Monospace,
|
||||
fontSize = 11.sp,
|
||||
),
|
||||
)
|
||||
}
|
||||
view.stderr?.let {
|
||||
Text(
|
||||
text = it,
|
||||
style =
|
||||
MaterialTheme.typography.bodySmall.copy(
|
||||
color = contentColor.copy(alpha = 0.6f),
|
||||
fontFamily = FontFamily.Monospace,
|
||||
fontSize = 11.sp,
|
||||
),
|
||||
)
|
||||
}
|
||||
view.exitCode?.let { code ->
|
||||
if (code != 0) {
|
||||
Text(
|
||||
text = stringResource(R.string.chat_tool_exit_code, code),
|
||||
style =
|
||||
MaterialTheme.typography.labelSmall.copy(
|
||||
color = statusColors.error,
|
||||
fontWeight = FontWeight.Medium,
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// ── File diff ──
|
||||
if (view.inlineDiff != null) {
|
||||
DiffViewCard(
|
||||
diffText = view.inlineDiff,
|
||||
filePath = view.diffPath,
|
||||
)
|
||||
}
|
||||
|
||||
// ── Search hits ──
|
||||
if (!view.searchHits.isNullOrEmpty()) {
|
||||
view.detailLabel?.let { label ->
|
||||
Text(
|
||||
text = label,
|
||||
style =
|
||||
MaterialTheme.typography.labelSmall.copy(
|
||||
color = contentColor.copy(alpha = 0.6f),
|
||||
fontWeight = FontWeight.Bold,
|
||||
),
|
||||
)
|
||||
}
|
||||
view.searchHits.forEach { hit ->
|
||||
Column(modifier = Modifier.padding(top = 2.dp)) {
|
||||
if (hit.title.isNotEmpty()) {
|
||||
Text(
|
||||
text = hit.title,
|
||||
style =
|
||||
MaterialTheme.typography.bodySmall.copy(
|
||||
color = contentColor.copy(alpha = 0.9f),
|
||||
fontWeight = FontWeight.Medium,
|
||||
),
|
||||
)
|
||||
}
|
||||
if (hit.snippet.isNotEmpty()) {
|
||||
Text(
|
||||
text = hit.snippet,
|
||||
style =
|
||||
MaterialTheme.typography.bodySmall.copy(
|
||||
color = contentColor.copy(alpha = 0.6f),
|
||||
fontSize = 11.sp,
|
||||
),
|
||||
maxLines = 3,
|
||||
overflow = TextOverflow.Ellipsis,
|
||||
)
|
||||
}
|
||||
if (hit.url.isNotEmpty()) {
|
||||
Text(
|
||||
text = "🔗 ${hit.url}",
|
||||
style =
|
||||
MaterialTheme.typography.bodySmall.copy(
|
||||
color = contentColor.copy(alpha = 0.7f),
|
||||
fontSize = 11.sp,
|
||||
),
|
||||
maxLines = 1,
|
||||
overflow = TextOverflow.Ellipsis,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ── Plain detail body ──
|
||||
if (view.detail.isNotBlank() && view.inlineDiff == null) {
|
||||
Text(
|
||||
text = view.detail,
|
||||
style =
|
||||
MaterialTheme.typography.bodySmall.copy(
|
||||
color = contentColor.copy(alpha = 0.9f),
|
||||
fontSize = 12.sp,
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// ── Duration footer ──
|
||||
view.durationLabel?.let {
|
||||
Text(
|
||||
text = "Duration: $it",
|
||||
style =
|
||||
MaterialTheme.typography.labelSmall.copy(
|
||||
color = contentColor.copy(alpha = 0.5f),
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Composable
|
||||
private fun HeaderRow(
|
||||
message: ChatMessage,
|
||||
|
||||
File diff suppressed because it is too large.
Load diff
@@ -47,10 +47,13 @@ object ToolViewBuilder {
|
||||
if (v is JsonNull) {
|
||||
return null
|
||||
}
|
||||
if (v !is JsonPrimitive || v.isString) {
|
||||
if (v !is JsonPrimitive) {
|
||||
return null
|
||||
}
|
||||
|
||||
// TS source does `Number(value)` — numeric STRINGS ("0.0", "12") are
|
||||
// accepted, matching the desktop engine and the gateway's habit of
|
||||
// shipping exit_code as a string or float.
|
||||
return v.content.toDoubleOrNull()
|
||||
}
|
||||
|
||||
@@ -383,6 +386,13 @@ object ToolViewBuilder {
|
||||
|
||||
// ── titles ───────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* The generic humanized title for a tool name ("fact_store" -> "Fact
|
||||
* Store"). Exposed so the view can drop the title from the collapsed
|
||||
* summary when it adds nothing over the header row's own tool name.
|
||||
*/
|
||||
internal fun genericTitleFor(toolName: String?): String = titleForTool(toolName ?: "tool")
|
||||
|
||||
private fun titleForTool(name: String): String {
|
||||
val normalized = name.replace(Regex("^browser_"), "").replace(Regex("^web_"), "")
|
||||
|
||||
@@ -946,12 +956,19 @@ object ToolViewBuilder {
|
||||
}
|
||||
}
|
||||
"memory" -> {
|
||||
when (firstString(args, listOf("action")).lowercase()) {
|
||||
"replace", "update" -> "Memory Updated"
|
||||
"remove", "delete" -> "Memory Removed"
|
||||
"add" -> "Memory Saved"
|
||||
else -> "Memory ${firstString(args, listOf("action"))}"
|
||||
}
|
||||
val action = firstString(args, listOf("action")).lowercase()
|
||||
val target = firstString(args, listOf("target"))
|
||||
val base =
|
||||
when (action) {
|
||||
"replace", "update" -> "Memory Updated"
|
||||
"remove", "delete" -> "Memory Removed"
|
||||
"add" -> "Memory Saved"
|
||||
else -> "Memory ${firstString(args, listOf("action"))}"
|
||||
}
|
||||
|
||||
// Keep the target visible — "Memory Saved (user)" — like the
|
||||
// pre-engine mobile summary did.
|
||||
if (target.isNotEmpty()) "$base ($target)" else base
|
||||
}
|
||||
"cronjob" -> "Cron ${firstString(args, listOf("action")).ifEmpty { "manage" }}"
|
||||
"edit_file", "patch", "write_file" -> {
|
||||
|
||||
Reference in new issue
Block a user