From 8c13ad5832764717541869cc52d3f10af48e8297 Mon Sep 17 00:00:00 2001 From: Clifford Ressel Date: Mon, 18 May 2026 16:41:48 -0400 Subject: [PATCH 1/2] fix(tui): highlight mcp history argument values --- nori-rs/tui/src/history_cell/mod.rs | 54 ++++++++++++++----- ..._highlights_argument_values_not_names.snap | 22 ++++++++ nori-rs/tui/src/history_cell/tests.rs | 47 ++++++++++++++++ 3 files changed, 111 insertions(+), 12 deletions(-) create mode 100644 nori-rs/tui/src/history_cell/snapshots/nori_tui__history_cell__tests__mcp_tool_call_highlights_argument_values_not_names.snap diff --git a/nori-rs/tui/src/history_cell/mod.rs b/nori-rs/tui/src/history_cell/mod.rs index 5854fd4c7..54acd00bf 100644 --- a/nori-rs/tui/src/history_cell/mod.rs +++ b/nori-rs/tui/src/history_cell/mod.rs @@ -1332,25 +1332,55 @@ impl HistoryCell for FinalMessageSeparator { } fn format_mcp_invocation<'a>(invocation: McpInvocation) -> Line<'a> { - let args_str = invocation - .arguments - .as_ref() - .map(|v: &serde_json::Value| { - // Use compact form to keep things short but readable. - serde_json::to_string(v).unwrap_or_else(|_| v.to_string()) - }) - .unwrap_or_default(); - - let invocation_spans = vec![ + let mut invocation_spans = vec![ invocation.server.clone().cyan(), ".".into(), invocation.tool.cyan(), "(".into(), - args_str.dim(), - ")".into(), ]; + if let Some(arguments) = invocation.arguments.as_ref() { + invocation_spans.extend(json_argument_spans(arguments)); + } + invocation_spans.push(")".into()); invocation_spans.into() } +fn json_argument_spans(value: &serde_json::Value) -> Vec> { + match value { + serde_json::Value::Null + | serde_json::Value::Bool(_) + | serde_json::Value::Number(_) + | serde_json::Value::String(_) => { + let text = serde_json::to_string(value).unwrap_or_else(|_| value.to_string()); + vec![text.cyan()] + } + serde_json::Value::Array(values) => { + let mut spans = vec!["[".dim()]; + for (idx, item) in values.iter().enumerate() { + if idx > 0 { + spans.push(",".dim()); + } + spans.extend(json_argument_spans(item)); + } + spans.push("]".dim()); + spans + } + serde_json::Value::Object(map) => { + let mut spans = vec!["{".dim()]; + for (idx, (key, item)) in map.iter().enumerate() { + if idx > 0 { + spans.push(",".dim()); + } + let key_text = serde_json::to_string(key).unwrap_or_else(|_| format!("\"{key}\"")); + spans.push(key_text.into()); + spans.push(":".dim()); + spans.extend(json_argument_spans(item)); + } + spans.push("}".dim()); + spans + } + } +} + #[cfg(test)] mod tests; diff --git a/nori-rs/tui/src/history_cell/snapshots/nori_tui__history_cell__tests__mcp_tool_call_highlights_argument_values_not_names.snap b/nori-rs/tui/src/history_cell/snapshots/nori_tui__history_cell__tests__mcp_tool_call_highlights_argument_values_not_names.snap new file mode 100644 index 000000000..add99fea5 --- /dev/null +++ b/nori-rs/tui/src/history_cell/snapshots/nori_tui__history_cell__tests__mcp_tool_call_highlights_argument_values_not_names.snap @@ -0,0 +1,22 @@ +--- +source: tui/src/history_cell/tests.rs +expression: styled_spans +--- +"•" Style::new().dim() +" " Style::new() +"Calling" Style::new().bold() +" " Style::new() +"search" Style::new().cyan() +"." Style::new() +"find_docs" Style::new().cyan() +"(" Style::new() +"{" Style::new().dim() +"\"query\"" Style::new() +":" Style::new().dim() +"\"ratatui styling\"" Style::new().cyan() +"," Style::new().dim() +"\"limit\"" Style::new() +":" Style::new().dim() +"3" Style::new().cyan() +"}" Style::new().dim() +")" Style::new() diff --git a/nori-rs/tui/src/history_cell/tests.rs b/nori-rs/tui/src/history_cell/tests.rs index 05fea607d..404f38067 100644 --- a/nori-rs/tui/src/history_cell/tests.rs +++ b/nori-rs/tui/src/history_cell/tests.rs @@ -8,6 +8,8 @@ use codex_core::config::ConfigToml; use codex_protocol::parse_command::ParsedCommand; use dirs::home_dir; use pretty_assertions::assert_eq; +use ratatui::style::Color; +use ratatui::style::Style; use serde_json::json; use codex_core::protocol::ExecCommandSource; @@ -87,6 +89,51 @@ fn active_mcp_tool_call_snapshot() { insta::assert_snapshot!(rendered); } +#[test] +fn mcp_tool_call_highlights_argument_values_not_names() { + let invocation = McpInvocation { + server: "search".into(), + tool: "find_docs".into(), + arguments: Some(json!({ + "query": "ratatui styling", + "limit": 3, + })), + }; + + let cell = new_active_mcp_tool_call("call-values".into(), invocation, false); + let rendered = cell.display_lines(120); + let spans = &rendered[0].spans; + let styled_spans = spans + .iter() + .map(|span| format!("{:?} {:?}", span.content, span.style)) + .collect::>() + .join("\n"); + + insta::assert_snapshot!(styled_spans); + + let query_name = spans + .iter() + .find(|span| span.content.as_ref() == "\"query\"") + .expect("argument name should render as its own span"); + let query_value = spans + .iter() + .find(|span| span.content.as_ref() == "\"ratatui styling\"") + .expect("argument value should render as its own span"); + let limit_name = spans + .iter() + .find(|span| span.content.as_ref() == "\"limit\"") + .expect("argument name should render as its own span"); + let limit_value = spans + .iter() + .find(|span| span.content.as_ref() == "3") + .expect("numeric argument value should render as its own span"); + + assert_eq!(query_name.style, Style::default()); + assert_eq!(limit_name.style, Style::default()); + assert_eq!(query_value.style.fg, Some(Color::Cyan)); + assert_eq!(limit_value.style.fg, Some(Color::Cyan)); +} + #[test] fn completed_mcp_tool_call_success_snapshot() { let invocation = McpInvocation { From 85a338b708de67da34501c61459d8d5e3ce8b3d1 Mon Sep 17 00:00:00 2001 From: Clifford Ressel Date: Mon, 18 May 2026 17:04:18 -0400 Subject: [PATCH 2/2] fix(tui): highlight session config values only --- nori-rs/tui/src/history_cell/mod.rs | 54 +++-------- ..._highlights_argument_values_not_names.snap | 22 ----- nori-rs/tui/src/history_cell/tests.rs | 47 --------- .../tui/src/nori/session_config_history.rs | 96 ++++++++++++++++++- ...tory_cells_highlight_values_not_names.snap | 9 ++ 5 files changed, 112 insertions(+), 116 deletions(-) delete mode 100644 nori-rs/tui/src/history_cell/snapshots/nori_tui__history_cell__tests__mcp_tool_call_highlights_argument_values_not_names.snap create mode 100644 nori-rs/tui/src/nori/snapshots/nori_tui__nori__session_config_history__tests__history_cells_highlight_values_not_names.snap diff --git a/nori-rs/tui/src/history_cell/mod.rs b/nori-rs/tui/src/history_cell/mod.rs index 54acd00bf..5854fd4c7 100644 --- a/nori-rs/tui/src/history_cell/mod.rs +++ b/nori-rs/tui/src/history_cell/mod.rs @@ -1332,55 +1332,25 @@ impl HistoryCell for FinalMessageSeparator { } fn format_mcp_invocation<'a>(invocation: McpInvocation) -> Line<'a> { - let mut invocation_spans = vec![ + let args_str = invocation + .arguments + .as_ref() + .map(|v: &serde_json::Value| { + // Use compact form to keep things short but readable. + serde_json::to_string(v).unwrap_or_else(|_| v.to_string()) + }) + .unwrap_or_default(); + + let invocation_spans = vec![ invocation.server.clone().cyan(), ".".into(), invocation.tool.cyan(), "(".into(), + args_str.dim(), + ")".into(), ]; - if let Some(arguments) = invocation.arguments.as_ref() { - invocation_spans.extend(json_argument_spans(arguments)); - } - invocation_spans.push(")".into()); invocation_spans.into() } -fn json_argument_spans(value: &serde_json::Value) -> Vec> { - match value { - serde_json::Value::Null - | serde_json::Value::Bool(_) - | serde_json::Value::Number(_) - | serde_json::Value::String(_) => { - let text = serde_json::to_string(value).unwrap_or_else(|_| value.to_string()); - vec![text.cyan()] - } - serde_json::Value::Array(values) => { - let mut spans = vec!["[".dim()]; - for (idx, item) in values.iter().enumerate() { - if idx > 0 { - spans.push(",".dim()); - } - spans.extend(json_argument_spans(item)); - } - spans.push("]".dim()); - spans - } - serde_json::Value::Object(map) => { - let mut spans = vec!["{".dim()]; - for (idx, (key, item)) in map.iter().enumerate() { - if idx > 0 { - spans.push(",".dim()); - } - let key_text = serde_json::to_string(key).unwrap_or_else(|_| format!("\"{key}\"")); - spans.push(key_text.into()); - spans.push(":".dim()); - spans.extend(json_argument_spans(item)); - } - spans.push("}".dim()); - spans - } - } -} - #[cfg(test)] mod tests; diff --git a/nori-rs/tui/src/history_cell/snapshots/nori_tui__history_cell__tests__mcp_tool_call_highlights_argument_values_not_names.snap b/nori-rs/tui/src/history_cell/snapshots/nori_tui__history_cell__tests__mcp_tool_call_highlights_argument_values_not_names.snap deleted file mode 100644 index add99fea5..000000000 --- a/nori-rs/tui/src/history_cell/snapshots/nori_tui__history_cell__tests__mcp_tool_call_highlights_argument_values_not_names.snap +++ /dev/null @@ -1,22 +0,0 @@ ---- -source: tui/src/history_cell/tests.rs -expression: styled_spans ---- -"•" Style::new().dim() -" " Style::new() -"Calling" Style::new().bold() -" " Style::new() -"search" Style::new().cyan() -"." Style::new() -"find_docs" Style::new().cyan() -"(" Style::new() -"{" Style::new().dim() -"\"query\"" Style::new() -":" Style::new().dim() -"\"ratatui styling\"" Style::new().cyan() -"," Style::new().dim() -"\"limit\"" Style::new() -":" Style::new().dim() -"3" Style::new().cyan() -"}" Style::new().dim() -")" Style::new() diff --git a/nori-rs/tui/src/history_cell/tests.rs b/nori-rs/tui/src/history_cell/tests.rs index 404f38067..05fea607d 100644 --- a/nori-rs/tui/src/history_cell/tests.rs +++ b/nori-rs/tui/src/history_cell/tests.rs @@ -8,8 +8,6 @@ use codex_core::config::ConfigToml; use codex_protocol::parse_command::ParsedCommand; use dirs::home_dir; use pretty_assertions::assert_eq; -use ratatui::style::Color; -use ratatui::style::Style; use serde_json::json; use codex_core::protocol::ExecCommandSource; @@ -89,51 +87,6 @@ fn active_mcp_tool_call_snapshot() { insta::assert_snapshot!(rendered); } -#[test] -fn mcp_tool_call_highlights_argument_values_not_names() { - let invocation = McpInvocation { - server: "search".into(), - tool: "find_docs".into(), - arguments: Some(json!({ - "query": "ratatui styling", - "limit": 3, - })), - }; - - let cell = new_active_mcp_tool_call("call-values".into(), invocation, false); - let rendered = cell.display_lines(120); - let spans = &rendered[0].spans; - let styled_spans = spans - .iter() - .map(|span| format!("{:?} {:?}", span.content, span.style)) - .collect::>() - .join("\n"); - - insta::assert_snapshot!(styled_spans); - - let query_name = spans - .iter() - .find(|span| span.content.as_ref() == "\"query\"") - .expect("argument name should render as its own span"); - let query_value = spans - .iter() - .find(|span| span.content.as_ref() == "\"ratatui styling\"") - .expect("argument value should render as its own span"); - let limit_name = spans - .iter() - .find(|span| span.content.as_ref() == "\"limit\"") - .expect("argument name should render as its own span"); - let limit_value = spans - .iter() - .find(|span| span.content.as_ref() == "3") - .expect("numeric argument value should render as its own span"); - - assert_eq!(query_name.style, Style::default()); - assert_eq!(limit_name.style, Style::default()); - assert_eq!(query_value.style.fg, Some(Color::Cyan)); - assert_eq!(limit_value.style.fg, Some(Color::Cyan)); -} - #[test] fn completed_mcp_tool_call_success_snapshot() { let invocation = McpInvocation { diff --git a/nori-rs/tui/src/nori/session_config_history.rs b/nori-rs/tui/src/nori/session_config_history.rs index 20da90d8a..6c832bdbb 100644 --- a/nori-rs/tui/src/nori/session_config_history.rs +++ b/nori-rs/tui/src/nori/session_config_history.rs @@ -5,6 +5,7 @@ use std::collections::BTreeMap; use nori_acp as acp; use ratatui::style::Stylize; use ratatui::text::Line; +use ratatui::text::Span; use crate::history_cell::PlainHistoryCell; @@ -59,7 +60,7 @@ pub(crate) fn new_agent_options_initial_history_cell( if index > 0 { line.push(", ".into()); } - line.push(format!("{}={}", value.name, value.value).cyan().bold()); + line.extend(option_assignment_spans(&value.name, &value.value)); } line.push(" (/config to change)".dim()); @@ -89,7 +90,7 @@ pub(crate) fn new_agent_options_history_cell( if index > 0 { line.push(", ".into()); } - line.push(format!("{}={}", change.name, change.value).cyan().bold()); + line.extend(option_assignment_spans(&change.name, &change.value)); } PlainHistoryCell::new(vec![Line::from(line)]) @@ -105,11 +106,21 @@ pub(crate) fn new_agent_option_set_history_cell( } else { agent_display_name }; - PlainHistoryCell::new(vec![Line::from(vec![ + let mut line = vec![ "• ".dim(), format!("{agent_display_name} option set: ").into(), - format!("{option_name}={value_name}").cyan().bold(), - ])]) + ]; + line.extend(option_assignment_spans(option_name, value_name)); + + PlainHistoryCell::new(vec![Line::from(line)]) +} + +fn option_assignment_spans(name: &str, value: &str) -> Vec> { + vec![ + name.to_string().into(), + "=".into(), + value.to_string().cyan().bold(), + ] } fn display_value(option: &acp::SessionConfigOption) -> Option { @@ -136,3 +147,78 @@ fn display_value(option: &acp::SessionConfigOption) -> Option], + expected_name: &str, + expected_value: &str, + ) { + let name = spans + .iter() + .find(|span| span.content.as_ref() == expected_name) + .expect("option name should be its own span"); + let separator = spans + .iter() + .find(|span| span.content.as_ref() == "=") + .expect("separator should be its own span"); + let value = spans + .iter() + .find(|span| span.content.as_ref() == expected_value) + .expect("option value should be its own span"); + + assert_eq!(name.style, Style::default()); + assert_eq!(separator.style, Style::default()); + assert_eq!(value.style.fg, Some(Color::Cyan)); + assert!(value.style.add_modifier.contains(Modifier::BOLD)); + } + + fn spans_to_style_snapshot(spans: &[ratatui::text::Span<'static>]) -> String { + spans + .iter() + .map(|span| format!("{:?} {:?}", span.content, span.style)) + .collect::>() + .join("\n") + } +} diff --git a/nori-rs/tui/src/nori/snapshots/nori_tui__nori__session_config_history__tests__history_cells_highlight_values_not_names.snap b/nori-rs/tui/src/nori/snapshots/nori_tui__nori__session_config_history__tests__history_cells_highlight_values_not_names.snap new file mode 100644 index 000000000..8b94e678b --- /dev/null +++ b/nori-rs/tui/src/nori/snapshots/nori_tui__nori__session_config_history__tests__history_cells_highlight_values_not_names.snap @@ -0,0 +1,9 @@ +--- +source: tui/src/nori/session_config_history.rs +expression: "spans_to_style_snapshot(&lines[0].spans)" +--- +"• " Style::new().dim() +"Claude Code option set: " Style::new() +"Model" Style::new() +"=" Style::new() +"Opus 4.6" Style::new().cyan().bold()