From 0e4b69dd201b36a3398d1932a89a30632255c252 Mon Sep 17 00:00:00 2001 From: iris Date: Sun, 19 Jul 2026 18:37:14 +0200 Subject: [PATCH] feat(#2196): move Write/Edit diff body to backend, drop JS branch - fmt_builtin_tool: Edit gets its own arm computing `-N +M` line counts in _summary (was shared with Read/Write as bare file-path). - is_rich_tool: drop Write (content is huge/one-sided; flat _summary row is correct); Edit stays rich since it has an old/new diff. - rich_tool_body: now returns Option<(String, &'static str)> where the second field is the body type ('diff' or 'plain'). Edit arm builds the '-'/'+ ' prefixed diff body; mcp__bash__run gets type 'plain'. - enrich_tool_use_entry: stamps both _body and _body_type. Frontend (app.js): - Remove the Write/Edit branch from renderRichToolUse (~25 lines). - Generic _body path now dispatches on _body_type: 'diff' -> api.detailsDiff (colour-coded spans), default -> api.details. No tool-specific JS remains for file diff rendering. --- frontend/packages/agent/src/app.js | 41 ++++--------------- hive-agent/src/stream_enrich.rs | 66 +++++++++++++++++++++++++----- 2 files changed, 64 insertions(+), 43 deletions(-) diff --git a/frontend/packages/agent/src/app.js b/frontend/packages/agent/src/app.js index 7550c321..3b3362c3 100644 --- a/frontend/packages/agent/src/app.js +++ b/frontend/packages/agent/src/app.js @@ -1483,33 +1483,6 @@ window.marked = marked; const name = c.name || ''; const input = c.input || {}; const icon = c._icon || '🔧'; - if (name === 'Write' || name === 'Edit') { - const path = input.file_path || '?'; - let body; - let plus = 0; - let minus = 0; - if (name === 'Write') { - const content = String(input.content || ''); - const lines = content.split('\n'); - plus = lines.length; - body = lines.map(l => '+ ' + l).join('\n'); - } else { - const oldLines = String(input.old_string || '').split('\n'); - const newLines = String(input.new_string || '').split('\n'); - minus = oldLines.length; - plus = newLines.length; - body = oldLines.map(l => '- ' + l).join('\n') - + '\n' - + newLines.map(l => '+ ' + l).join('\n'); - } - // The tool icon goes in the shared `.row-glyph` cell (4th arg) so it - // lines up with flat-row icons; the summary text carries no - // directional `→` (the row's cyan colour signals "outbound tool" and - // the CSS disclosure caret leads the text). - const summary = name + ' ' + path + ' · ' - + (minus ? '-' + minus + ' ' : '') + '+' + plus; - return api.detailsDiff('tool-use', summary, body, icon); - } // Message-bearing tools render default-open with a markdown body so // the operator sees the content without an extra click. send / ask // address a target; answer attaches to an existing question id. @@ -1559,11 +1532,15 @@ window.marked = marked; 'answer #' + id + (lines > 1 ? ` · ${lines}L` : ''), a, icon); } - // Generic plain-text body: the backend stamps `_body` for tools whose - // expandable content is plain text (e.g. mcp__bash__run full command). - // Render as a collapsible details row without any tool-specific logic. - if (c._body) { - return api.details('tool-use', c._summary || name || '?', c._body, icon); + // Generic backend-computed body: the backend stamps `_body` + `_body_type` + // for tools whose expandable content is pre-computable (Edit diff, + // mcp__bash__run command). Dispatch on type — no tool-specific JS needed. + if (c._body != null) { + const summary = c._summary || name || '?'; + if (c._body_type === 'diff') { + return api.detailsDiff('tool-use', summary, c._body, icon); + } + return api.details('tool-use', summary, c._body, icon); } return null; } diff --git a/hive-agent/src/stream_enrich.rs b/hive-agent/src/stream_enrich.rs index c94ab281..3789e553 100644 --- a/hive-agent/src/stream_enrich.rs +++ b/hive-agent/src/stream_enrich.rs @@ -239,8 +239,9 @@ fn enrich_tool_use_entry(entry: &mut Value) { if rich { obj.insert("_category".to_owned(), json!("rich")); } - if let Some(b) = body { + if let Some((b, bt)) = body { obj.insert("_body".to_owned(), json!(b)); + obj.insert("_body_type".to_owned(), json!(bt)); } } @@ -255,8 +256,7 @@ fn enrich_tool_use_entry(entry: &mut Value) { fn is_rich_tool(name: &str) -> bool { matches!( name, - "Write" - | "Edit" + "Edit" | "mcp__bash__run" | "mcp__hyperhive__send" | "mcp__hyperhive__ask" @@ -264,14 +264,42 @@ fn is_rich_tool(name: &str) -> bool { ) } -/// Pre-compute the expandable body text for tools that have one. +/// Pre-compute the expandable body for rich tool entries. /// -/// Returns `Some(body)` when the tool has a meaningful multi-line body that -/// the frontend can display in a `
` block without tool-specific JS. -/// `None` for tools whose body is computed client-side (diffs, markdown) or -/// that have no body at all. -fn rich_tool_body(name: &str, input: &Value) -> Option { +/// Returns `Some((body, body_type))` where `body_type` tells the frontend +/// which renderer to use: +/// - `"diff"` → `api.detailsDiff` (colour-coded `+`/`-` lines) +/// - `"plain"` → `api.details` (plain `
` block)
+///
+/// Returns `None` for tools whose body is inherently client-side (markdown
+/// rendering, DOM forms) or that have no body at all.
+fn rich_tool_body(name: &str, input: &Value) -> Option<(String, &'static str)> {
     match name {
+        // Edit diff: old lines prefixed `- `, new lines prefixed `+ `.
+        // The summary already carries `-N +M` counts (from `fmt_builtin_tool`),
+        // so the body is the full colour-coded diff.
+        "Edit" => {
+            let old = input
+                .get("old_string")
+                .and_then(Value::as_str)
+                .unwrap_or("");
+            let new = input
+                .get("new_string")
+                .and_then(Value::as_str)
+                .unwrap_or("");
+            let mut body = String::new();
+            for line in old.lines() {
+                body.push_str("- ");
+                body.push_str(line);
+                body.push('\n');
+            }
+            for line in new.lines() {
+                body.push_str("+ ");
+                body.push_str(line);
+                body.push('\n');
+            }
+            Some((body, "diff"))
+        }
         // Full bash command — first line is already in `_summary`; the full
         // `cmd` (prefixed with `$ `) is the body so multi-line scripts are
         // readable on expand.
@@ -280,7 +308,7 @@ fn rich_tool_body(name: &str, input: &Value) -> Option {
             if cmd.is_empty() {
                 None
             } else {
-                Some(format!("$ {cmd}"))
+                Some((format!("$ {cmd}"), "plain"))
             }
         }
         _ => None,
@@ -365,7 +393,23 @@ fn fmt_tool_use(name: &str, input: &Value) -> String {
 /// unknown tool that doesn't carry a known MCP server prefix.
 fn fmt_builtin_tool(name: &str, short: &str, input: &Value) -> String {
     match name {
-        "Read" | "Write" | "Edit" => format!("{short} {}", sv(input, "file_path")),
+        "Read" | "Write" => format!("{short} {}", sv(input, "file_path")),
+        "Edit" => {
+            let path = sv(input, "file_path");
+            let old_n = input
+                .get("old_string")
+                .and_then(Value::as_str)
+                .unwrap_or("")
+                .lines()
+                .count();
+            let new_n = input
+                .get("new_string")
+                .and_then(Value::as_str)
+                .unwrap_or("")
+                .lines()
+                .count();
+            format!("{short} {path} · -{old_n} +{new_n}")
+        }
         "Glob" | "Grep" => format!("{short} {}", sv(input, "pattern")),
         "Bash" => {
             let bg = if input