From d365f8f6fd4b286ad64b5525c5dd569a19def29a Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Thu, 21 May 2026 00:59:38 +0200 Subject: [PATCH] feat(config): Add `error_style` override for tool error responses Tools can now declare a dedicated `error_style` block in their config that controls how a failed tool response is rendered, independently of the shared `style` block that governs both request and response display. ```toml [conversation.tools.my_tool.error_style] inline_results = "full" results_file_link = "off" ``` When `error_style` is absent the renderer falls back to the response side of the existing `style` config, so the change is fully backwards compatible. Signed-off-by: Jean Mertz --- crates/jp_cli/src/cmd/conversation/print.rs | 16 ++- crates/jp_cli/src/cmd/query/tool/builtins.rs | 6 +- .../jp_cli/src/cmd/query/tool/coordinator.rs | 10 +- .../jp_cli/src/cmd/query/turn_loop_tests.rs | 6 +- crates/jp_cli/src/render/turn.rs | 17 ++- crates/jp_config/src/conversation/tool.rs | 3 +- .../jp_config/src/conversation/tool/style.rs | 132 +++++++++++++++++- .../src/conversation/tool/style_tests.rs | 57 ++++++++ .../jp_config__tests__app_config_fields.snap | 2 + ...ig__tests__partial_app_config_default.snap | 4 + ...ts__partial_app_config_default_values.snap | 4 + ...s__partial_app_config_empty_serialize.snap | 4 + 12 files changed, 242 insertions(+), 19 deletions(-) diff --git a/crates/jp_cli/src/cmd/conversation/print.rs b/crates/jp_cli/src/cmd/conversation/print.rs index 19e30f5b..91e5a5da 100644 --- a/crates/jp_cli/src/cmd/conversation/print.rs +++ b/crates/jp_cli/src/cmd/conversation/print.rs @@ -1,5 +1,7 @@ use jp_config::{ - conversation::tool::style::{self, DisplayStyleConfig, InlineResults, ParametersStyle}, + conversation::tool::style::{ + self, DisplayStyleConfig, ErrorStyleConfig, InlineResults, ParametersStyle, + }, style::{reasoning::ReasoningDisplayConfig, typewriter::DelayDuration}, }; use jp_workspace::ConversationHandle; @@ -16,6 +18,10 @@ const BRIEF_TOOL_STYLE: DisplayStyleConfig = DisplayStyleConfig { parameters: ParametersStyle::Off, inline_results: InlineResults::Off, results_file_link: style::LinkStyle::Off, + error: ErrorStyleConfig { + inline_results: None, + results_file_link: None, + }, }; /// Chat-mode tool display style: tool calls are fully hidden. @@ -24,6 +30,10 @@ const CHAT_TOOL_STYLE: DisplayStyleConfig = DisplayStyleConfig { parameters: ParametersStyle::Off, inline_results: InlineResults::Off, results_file_link: style::LinkStyle::Off, + error: ErrorStyleConfig { + inline_results: None, + results_file_link: None, + }, }; /// Full-mode tool display style: everything visible, nothing truncated. @@ -32,6 +42,10 @@ const FULL_TOOL_STYLE: DisplayStyleConfig = DisplayStyleConfig { parameters: ParametersStyle::Json, inline_results: InlineResults::Full, results_file_link: style::LinkStyle::Full, + error: ErrorStyleConfig { + inline_results: None, + results_file_link: None, + }, }; #[derive(Debug, clap::Args)] diff --git a/crates/jp_cli/src/cmd/query/tool/builtins.rs b/crates/jp_cli/src/cmd/query/tool/builtins.rs index f070b417..85421aca 100644 --- a/crates/jp_cli/src/cmd/query/tool/builtins.rs +++ b/crates/jp_cli/src/cmd/query/tool/builtins.rs @@ -2,7 +2,10 @@ use indexmap::IndexMap; use jp_config::conversation::tool::{ Enable, PartialOneOrManyTypes, PartialToolConfig, PartialToolParameterConfig, RunMode, ToolSource, - style::{InlineResults, LinkStyle, ParametersStyle, PartialDisplayStyleConfig}, + style::{ + InlineResults, LinkStyle, ParametersStyle, PartialDisplayStyleConfig, + PartialErrorStyleConfig, + }, }; pub fn all() -> IndexMap { @@ -34,6 +37,7 @@ pub fn describe_tools() -> PartialToolConfig { inline_results: Some(InlineResults::Off), results_file_link: Some(LinkStyle::Off), parameters: Some(ParametersStyle::Off), + error: PartialErrorStyleConfig::default(), }), ..Default::default() } diff --git a/crates/jp_cli/src/cmd/query/tool/coordinator.rs b/crates/jp_cli/src/cmd/query/tool/coordinator.rs index 649e88e5..174aeb2f 100644 --- a/crates/jp_cli/src/cmd/query/tool/coordinator.rs +++ b/crates/jp_cli/src/cmd/query/tool/coordinator.rs @@ -1038,13 +1038,14 @@ impl ToolCoordinator { .get(&index) .map(|t| t.tool_name.clone()) .unwrap_or_default(); + let is_error = response.result.is_err(); let (inline_results, results_file_link) = self .tools_config .get(&tool_name) .map(|c| { ( - c.style().inline_results.clone(), - c.style().results_file_link.clone(), + c.style().inline_results(is_error).clone(), + c.style().results_file_link(is_error).clone(), ) }) .unwrap_or_default(); @@ -1252,13 +1253,14 @@ impl ToolCoordinator { ) { match result { ExecutorResult::Completed(response) => { + let is_error = response.result.is_err(); let (inline_results, results_file_link) = self .tools_config .get(&tool.tool_name) .map(|c| { ( - c.style().inline_results.clone(), - c.style().results_file_link.clone(), + c.style().inline_results(is_error).clone(), + c.style().results_file_link(is_error).clone(), ) }) .unwrap_or_default(); diff --git a/crates/jp_cli/src/cmd/query/turn_loop_tests.rs b/crates/jp_cli/src/cmd/query/turn_loop_tests.rs index 2ff09625..6ea715b9 100644 --- a/crates/jp_cli/src/cmd/query/turn_loop_tests.rs +++ b/crates/jp_cli/src/cmd/query/turn_loop_tests.rs @@ -16,7 +16,7 @@ use jp_config::{ conversation::tool::{ CommandConfigOrString, PartialCommandConfigOrString, PartialToolConfig, QuestionConfig, QuestionTarget, RunMode, ToolConfig, ToolSource, - style::{DisplayStyleConfig, InlineResults, LinkStyle, ParametersStyle}, + style::{DisplayStyleConfig, ErrorStyleConfig, InlineResults, LinkStyle, ParametersStyle}, }, model::id::{self, Name, PartialModelIdConfig, ProviderId}, }; @@ -2634,6 +2634,10 @@ async fn test_parallel_tool_calls_rendered_atomically() { inline_results: InlineResults::Off, results_file_link: LinkStyle::Off, parameters: ParametersStyle::FunctionCall, + error: ErrorStyleConfig { + inline_results: None, + results_file_link: None, + }, }); // Configure tools with FunctionCall style for readable output. diff --git a/crates/jp_cli/src/render/turn.rs b/crates/jp_cli/src/render/turn.rs index 48ca5eff..42e94ff2 100644 --- a/crates/jp_cli/src/render/turn.rs +++ b/crates/jp_cli/src/render/turn.rs @@ -131,16 +131,15 @@ impl TurnRenderer { let name = self.tool_names.get(&resp.id); let default_style = &self.tools_config.defaults.style; let tool_cfg = name.and_then(|n| self.tools_config.get(n)); - let style = tool_cfg - .as_ref() - .map_or(default_style, ToolConfigWithDefaults::style); + let tool_style = tool_cfg.as_ref().map_or(default_style, |c| c.style()); + let is_error = resp.result.is_err(); + let hidden = tool_style.hidden; + let inline_results = tool_style.inline_results(is_error); + let results_file_link = tool_style.results_file_link(is_error); - if !style.hidden { - self.tool.render_result( - resp, - &style.inline_results, - &style.results_file_link, - ); + if !hidden { + self.tool + .render_result(resp, inline_results, results_file_link); } } diff --git a/crates/jp_config/src/conversation/tool.rs b/crates/jp_config/src/conversation/tool.rs index f2a584db..86b80564 100644 --- a/crates/jp_config/src/conversation/tool.rs +++ b/crates/jp_config/src/conversation/tool.rs @@ -334,7 +334,8 @@ pub struct ToolConfig { /// How to display the results of the tool in the terminal. /// - /// Overrides the global default. + /// Overrides the global default. The error overlay lives at + /// `style.error.*` (see [`DisplayStyleConfig::error`]). #[setting(nested)] pub style: Option, diff --git a/crates/jp_config/src/conversation/tool/style.rs b/crates/jp_config/src/conversation/tool/style.rs index c520d380..28665242 100644 --- a/crates/jp_config/src/conversation/tool/style.rs +++ b/crates/jp_config/src/conversation/tool/style.rs @@ -1,4 +1,23 @@ //! Display style configuration for tools. +//! +//! User-facing shape under `[style]`: +//! +//! ```toml +//! [tools.my_tool.style] +//! hidden = false # applies to both request and response +//! parameters = "json" # how arguments are rendered on the call +//! inline_results = "full" # how successful results are rendered +//! results_file_link = "osc8" +//! +//! [tools.my_tool.style.error] +//! inline_results = "full" # overrides on top of `style` for failed results +//! ``` +//! +//! [`ErrorStyleConfig`] (`error`) is a per-field overlay applied when the +//! tool result is `Err`. Unset overlay fields take their value from the +//! matching top-level field. The overlay is typed separately so the +//! error path can grow its own fields (severity styling, alternate +//! sinks, etc.) independently of the success path. use std::{fmt, num::ParseIntError, str::FromStr}; @@ -11,10 +30,12 @@ use crate::{ conversation::tool::CommandConfigOrString, delta::{PartialConfigDelta, delta_opt}, fill::FillDefaults, - partial::{ToPartial, partial_opt}, + partial::{ToPartial, partial_opt, partial_opts}, }; /// Display style configuration. +/// +/// See the [module-level docs](self) for the user-facing TOML shape. #[derive(Debug, Clone, PartialEq, Config)] #[config(rename_all = "snake_case")] pub struct DisplayStyleConfig { @@ -49,16 +70,61 @@ pub struct DisplayStyleConfig { /// - ``: Use a custom command to format the parameters. #[setting(default)] pub parameters: ParametersStyle, + + /// Per-field overrides applied when the tool result is `Err`. Unset + /// overlay fields take their value from the matching top-level field. + /// + /// Sub-fields appear under `[style.error]` in TOML. + #[setting(nested)] + pub error: ErrorStyleConfig, +} + +impl DisplayStyleConfig { + /// Return the effective [`InlineResults`] for a tool response. + /// + /// For error responses, the `style.error.inline_results` overlay is + /// applied on top of `style.inline_results`. Successful responses + /// read `style.inline_results` directly. + #[must_use] + pub fn inline_results(&self, is_error: bool) -> &InlineResults { + if is_error { + self.error + .inline_results + .as_ref() + .unwrap_or(&self.inline_results) + } else { + &self.inline_results + } + } + + /// Return the effective [`LinkStyle`] for the results-file link of a + /// tool response. + /// + /// For error responses, the `style.error.results_file_link` overlay + /// is applied on top of `style.results_file_link`. Successful + /// responses read `style.results_file_link` directly. + #[must_use] + pub fn results_file_link(&self, is_error: bool) -> &LinkStyle { + if is_error { + self.error + .results_file_link + .as_ref() + .unwrap_or(&self.results_file_link) + } else { + &self.results_file_link + } + } } impl AssignKeyValue for PartialDisplayStyleConfig { - fn assign(&mut self, kv: KvAssignment) -> AssignResult { + fn assign(&mut self, mut kv: KvAssignment) -> AssignResult { match kv.key_string().as_str() { "" => kv.try_merge_object(self)?, "hidden" => self.hidden = kv.try_some_bool()?, "inline_results" => self.inline_results = kv.try_some_from_str()?, "results_file_link" => self.results_file_link = kv.try_some_from_str()?, "parameters" => self.parameters = kv.try_some_from_str()?, + _ if kv.p("error") => self.error.assign(kv)?, _ => return missing_key(&kv), } @@ -73,6 +139,7 @@ impl PartialConfigDelta for PartialDisplayStyleConfig { inline_results: delta_opt(self.inline_results.as_ref(), next.inline_results), results_file_link: delta_opt(self.results_file_link.as_ref(), next.results_file_link), parameters: delta_opt(self.parameters.as_ref(), next.parameters), + error: self.error.delta(next.error), } } } @@ -84,6 +151,7 @@ impl FillDefaults for PartialDisplayStyleConfig { inline_results: self.inline_results.or(defaults.inline_results), results_file_link: self.results_file_link.or(defaults.results_file_link), parameters: self.parameters.or(defaults.parameters), + error: self.error.fill_from(defaults.error), } } } @@ -97,6 +165,66 @@ impl ToPartial for DisplayStyleConfig { inline_results: partial_opt(&self.inline_results, defaults.inline_results), results_file_link: partial_opt(&self.results_file_link, defaults.results_file_link), parameters: partial_opt(&self.parameters, defaults.parameters), + error: self.error.to_partial(), + } + } +} + +/// Per-field overrides for rendering failed tool call responses. +/// +/// Each field is optional. Callers resolve an unset field by reading the +/// matching field from the parent [`DisplayStyleConfig`]. +#[derive(Debug, Clone, PartialEq, Default, Config)] +#[config(rename_all = "snake_case")] +pub struct ErrorStyleConfig { + /// Override for [`DisplayStyleConfig::inline_results`]. + pub inline_results: Option, + + /// Override for [`DisplayStyleConfig::results_file_link`]. + pub results_file_link: Option, +} + +impl AssignKeyValue for PartialErrorStyleConfig { + fn assign(&mut self, kv: KvAssignment) -> AssignResult { + match kv.key_string().as_str() { + "" => kv.try_merge_object(self)?, + "inline_results" => self.inline_results = kv.try_some_from_str()?, + "results_file_link" => self.results_file_link = kv.try_some_from_str()?, + _ => return missing_key(&kv), + } + + Ok(()) + } +} + +impl PartialConfigDelta for PartialErrorStyleConfig { + fn delta(&self, next: Self) -> Self { + Self { + inline_results: delta_opt(self.inline_results.as_ref(), next.inline_results), + results_file_link: delta_opt(self.results_file_link.as_ref(), next.results_file_link), + } + } +} + +impl FillDefaults for PartialErrorStyleConfig { + fn fill_from(self, defaults: Self) -> Self { + Self { + inline_results: self.inline_results.or(defaults.inline_results), + results_file_link: self.results_file_link.or(defaults.results_file_link), + } + } +} + +impl ToPartial for ErrorStyleConfig { + fn to_partial(&self) -> Self::Partial { + let defaults = Self::Partial::default(); + + Self::Partial { + inline_results: partial_opts(self.inline_results.as_ref(), defaults.inline_results), + results_file_link: partial_opts( + self.results_file_link.as_ref(), + defaults.results_file_link, + ), } } } diff --git a/crates/jp_config/src/conversation/tool/style_tests.rs b/crates/jp_config/src/conversation/tool/style_tests.rs index 9df7617d..bdb557b7 100644 --- a/crates/jp_config/src/conversation/tool/style_tests.rs +++ b/crates/jp_config/src/conversation/tool/style_tests.rs @@ -1,6 +1,7 @@ use serde_json::from_str; use super::*; +use crate::assignment::KvAssignment; #[test] fn test_link_style_deserialization() { @@ -42,3 +43,59 @@ fn test_inline_results_deserialization() { InlineResults::Truncate(TruncateLines { lines: 5 }) ); } + +#[test] +fn test_assign_error_inline_results_only_sets_overlay_field() { + let mut partial = PartialDisplayStyleConfig::default(); + let kv = KvAssignment::try_from_cli("error.inline_results", "full").unwrap(); + partial.assign(kv).unwrap(); + + assert_eq!(partial.error.inline_results, Some(InlineResults::Full)); + assert_eq!(partial.error.results_file_link, None); + assert_eq!(partial.inline_results, None); + assert_eq!(partial.results_file_link, None); +} + +#[test] +fn test_assign_error_block_via_json() { + let mut partial = PartialDisplayStyleConfig::default(); + let kv = KvAssignment::try_from_cli( + "error:", + r#"{"inline_results":"off","results_file_link":"osc8"}"#, + ) + .unwrap(); + partial.assign(kv).unwrap(); + + assert_eq!(partial.error.inline_results, Some(InlineResults::Off)); + assert_eq!(partial.error.results_file_link, Some(LinkStyle::Osc8)); +} + +#[test] +fn test_error_overlay_falls_back_per_field() { + let style = DisplayStyleConfig { + hidden: false, + parameters: ParametersStyle::Json, + inline_results: InlineResults::Truncate(TruncateLines { lines: 5 }), + results_file_link: LinkStyle::Full, + error: ErrorStyleConfig { + inline_results: Some(InlineResults::Full), + results_file_link: None, + }, + }; + + // Field set on the overlay: overlay wins. + let il = style + .error + .inline_results + .as_ref() + .unwrap_or(&style.inline_results); + assert_eq!(*il, InlineResults::Full); + + // Field unset on the overlay: inherits from the top-level field. + let rfl = style + .error + .results_file_link + .as_ref() + .unwrap_or(&style.results_file_link); + assert_eq!(*rfl, LinkStyle::Full); +} diff --git a/crates/jp_config/src/snapshots/jp_config__tests__app_config_fields.snap b/crates/jp_config/src/snapshots/jp_config__tests__app_config_fields.snap index 30d3d6a0..e5d1a436 100644 --- a/crates/jp_config/src/snapshots/jp_config__tests__app_config_fields.snap +++ b/crates/jp_config/src/snapshots/jp_config__tests__app_config_fields.snap @@ -75,6 +75,8 @@ expression: "AppConfig::fields()" "conversation.tools.*.style.inline_results", "conversation.tools.*.style.parameters", "conversation.tools.*.style.results_file_link", + "conversation.tools.*.style.error.inline_results", + "conversation.tools.*.style.error.results_file_link", "conversation.title.generate.auto", "conversation.title.generate.model", "conversation.inquiry.assistant.model", diff --git a/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default.snap b/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default.snap index 5c1b4632..2c60a846 100644 --- a/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default.snap +++ b/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default.snap @@ -58,6 +58,10 @@ PartialAppConfig { inline_results: None, results_file_link: None, parameters: None, + error: PartialErrorStyleConfig { + inline_results: None, + results_file_link: None, + }, }, }, tools: {}, diff --git a/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default_values.snap b/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default_values.snap index afc746f8..4a061f24 100644 --- a/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default_values.snap +++ b/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default_values.snap @@ -112,6 +112,10 @@ Ok( inline_results: None, results_file_link: None, parameters: None, + error: PartialErrorStyleConfig { + inline_results: None, + results_file_link: None, + }, }, }, tools: {}, diff --git a/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_empty_serialize.snap b/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_empty_serialize.snap index 4241b266..cf9bea74 100644 --- a/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_empty_serialize.snap +++ b/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_empty_serialize.snap @@ -58,6 +58,10 @@ PartialAppConfig { inline_results: None, results_file_link: None, parameters: None, + error: PartialErrorStyleConfig { + inline_results: None, + results_file_link: None, + }, }, }, tools: {},