Skip to content

Commit 00a4d05

Browse files
authored
fix: simplify donut chart schema for OpenAI compatibility (#8303)
Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
1 parent 9858f82 commit 00a4d05

File tree

1 file changed

+57
-63
lines changed
  • crates/goose-mcp/src/autovisualiser

1 file changed

+57
-63
lines changed

crates/goose-mcp/src/autovisualiser/mod.rs

Lines changed: 57 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -277,16 +277,6 @@ pub struct SingleDonutChart {
277277
pub labels: Option<Vec<String>>,
278278
}
279279

280-
/// Donut chart data — a single chart object or an array of chart objects
281-
#[derive(Debug, Serialize, Deserialize, rmcp::schemars::JsonSchema)]
282-
#[serde(untagged)]
283-
pub enum DonutChartData {
284-
/// Single donut chart
285-
Single(SingleDonutChart),
286-
/// Multiple donut charts
287-
Multiple(Vec<SingleDonutChart>),
288-
}
289-
290280
impl SingleDonutChart {
291281
fn validate(&self) -> Result<(), ErrorData> {
292282
if self.values.is_empty() {
@@ -305,28 +295,21 @@ impl SingleDonutChart {
305295
}
306296
}
307297

308-
impl DonutChartData {
309-
fn validate(&self) -> Result<(), ErrorData> {
310-
match self {
311-
DonutChartData::Single(chart) => chart.validate(),
312-
DonutChartData::Multiple(charts) => {
313-
if charts.is_empty() {
314-
return Err(validation_err("charts array must not be empty"));
315-
}
316-
for chart in charts {
317-
chart.validate()?;
318-
}
319-
Ok(())
320-
}
321-
}
298+
fn validate_donut_charts(charts: &[SingleDonutChart]) -> Result<(), ErrorData> {
299+
if charts.is_empty() {
300+
return Err(validation_err("charts array must not be empty"));
301+
}
302+
for chart in charts {
303+
chart.validate()?;
322304
}
305+
Ok(())
323306
}
324307

325308
/// Parameters for render_donut tool
326309
#[derive(Debug, Serialize, Deserialize, rmcp::schemars::JsonSchema)]
327310
pub struct RenderDonutParams {
328-
/// The chart data (single chart object or array of chart objects)
329-
pub data: DonutChartData,
311+
/// The chart data as an array of chart objects. Use a single-element array for one chart.
312+
pub data: Vec<SingleDonutChart>,
330313
}
331314

332315
/// Treemap node structure
@@ -987,7 +970,8 @@ Example:
987970
#[tool(
988971
name = "render_donut",
989972
description = r#"show pie or donut charts for categorical data visualization.
990-
Supports single or multiple charts in a grid layout.
973+
Supports one or more charts in a grid layout.
974+
The `data` field must always be an array; pass a single-element array for one chart.
991975
992976
Each chart object must contain:
993977
- values: Array of numbers OR objects with 'label' and 'value'
@@ -996,20 +980,24 @@ Each chart object must contain:
996980
- labels: Optional array of labels (required when values are plain numbers)
997981
998982
Example single chart (labeled values):
999-
{
1000-
"values": [
1001-
{"label": "Marketing", "value": 25000},
1002-
{"label": "Development", "value": 35000}
1003-
],
1004-
"title": "Budget"
1005-
}
983+
[
984+
{
985+
"values": [
986+
{"label": "Marketing", "value": 25000},
987+
{"label": "Development", "value": 35000}
988+
],
989+
"title": "Budget"
990+
}
991+
]
1006992
1007993
Example single chart (parallel arrays):
1008-
{
1009-
"values": [45000, 38000],
1010-
"labels": ["Product A", "Product B"],
1011-
"type": "pie"
1012-
}
994+
[
995+
{
996+
"values": [45000, 38000],
997+
"labels": ["Product A", "Product B"],
998+
"type": "pie"
999+
}
1000+
]
10131001
10141002
Example multiple charts (array of chart objects):
10151003
[
@@ -1023,7 +1011,7 @@ Example multiple charts (array of chart objects):
10231011
params: Parameters<RenderDonutParams>,
10241012
) -> Result<CallToolResult, ErrorData> {
10251013
let inner = params.0;
1026-
inner.data.validate()?;
1014+
validate_donut_charts(&inner.data)?;
10271015
let data = validate_data_param(
10281016
&serde_json::to_value(inner).map_err(|e| {
10291017
ErrorData::new(
@@ -1035,15 +1023,21 @@ Example multiple charts (array of chart objects):
10351023
true,
10361024
)?;
10371025

1038-
let text_fallback = if data.is_array() {
1039-
let count = data.as_array().map(|a| a.len()).unwrap_or(0);
1040-
format!("donut/pie chart: {} chart(s)", count)
1041-
} else {
1042-
let title = data
1026+
let charts = data.as_array().ok_or_else(|| {
1027+
ErrorData::new(
1028+
ErrorCode::INVALID_PARAMS,
1029+
"The 'data' parameter must be an array.".to_string(),
1030+
None,
1031+
)
1032+
})?;
1033+
let text_fallback = if charts.len() == 1 {
1034+
let title = charts[0]
10431035
.get("title")
10441036
.and_then(|v| v.as_str())
10451037
.unwrap_or("Untitled");
10461038
format!("donut/pie chart: \"{}\"", title)
1039+
} else {
1040+
format!("donut/pie chart: {} chart(s)", charts.len())
10471041
};
10481042

10491043
let mut result = CallToolResult::structured(data);
@@ -1566,7 +1560,7 @@ mod tests {
15661560
async fn test_render_donut() {
15671561
let router = AutoVisualiserRouter::new();
15681562
let params = Parameters(RenderDonutParams {
1569-
data: DonutChartData::Single(SingleDonutChart {
1563+
data: vec![SingleDonutChart {
15701564
values: vec![
15711565
DonutDataItem::Number(30.0),
15721566
DonutDataItem::Number(40.0),
@@ -1575,7 +1569,7 @@ mod tests {
15751569
labels: Some(vec!["A".to_string(), "B".to_string(), "C".to_string()]),
15761570
title: None,
15771571
chart_type: None,
1578-
}),
1572+
}],
15791573
});
15801574

15811575
let result = router.render_donut(params).await;
@@ -1721,14 +1715,14 @@ mod donut_format_tests {
17211715

17221716
#[test]
17231717
fn labeled_values_single_chart() {
1724-
// {"data": {"values": [{"label": "A", "value": 10}, ...]}}
1718+
// {"data": [{"values": [{"label": "A", "value": 10}, ...]}]}
17251719
let input = json!({
1726-
"data": {
1720+
"data": [{
17271721
"values": [
17281722
{"label": "A", "value": 10},
17291723
{"label": "B", "value": 20}
17301724
]
1731-
}
1725+
}]
17321726
});
17331727
let result = round_trip(input);
17341728
assert!(
@@ -1740,12 +1734,12 @@ mod donut_format_tests {
17401734

17411735
#[test]
17421736
fn parallel_arrays_single_chart() {
1743-
// {"data": {"values": [10, 20], "labels": ["A", "B"]}}
1737+
// {"data": [{"values": [10, 20], "labels": ["A", "B"]}]}
17441738
let input = json!({
1745-
"data": {
1739+
"data": [{
17461740
"values": [10, 20],
17471741
"labels": ["A", "B"]
1748-
}
1742+
}]
17491743
});
17501744
let result = round_trip(input);
17511745
assert!(
@@ -1775,14 +1769,14 @@ mod donut_format_tests {
17751769
#[test]
17761770
fn labeled_values_with_title_and_type() {
17771771
let input = json!({
1778-
"data": {
1772+
"data": [{
17791773
"values": [
17801774
{"label": "Marketing", "value": 25000},
17811775
{"label": "Development", "value": 35000}
17821776
],
17831777
"title": "Budget",
17841778
"type": "pie"
1785-
}
1779+
}]
17861780
});
17871781
let result = round_trip(input);
17881782
assert!(
@@ -1910,31 +1904,31 @@ mod validation_tests {
19101904

19111905
#[test]
19121906
fn donut_rejects_empty_values() {
1913-
let data = DonutChartData::Single(SingleDonutChart {
1907+
let data = vec![SingleDonutChart {
19141908
values: vec![],
19151909
title: None,
19161910
chart_type: None,
19171911
labels: None,
1918-
});
1919-
assert!(data.validate().is_err());
1912+
}];
1913+
assert!(validate_donut_charts(&data).is_err());
19201914
}
19211915

19221916
#[test]
19231917
fn donut_rejects_mismatched_labels() {
1924-
let data = DonutChartData::Single(SingleDonutChart {
1918+
let data = vec![SingleDonutChart {
19251919
values: vec![DonutDataItem::Number(10.0), DonutDataItem::Number(20.0)],
19261920
title: None,
19271921
chart_type: None,
19281922
labels: Some(vec!["A".into()]), // 1 label but 2 values
1929-
});
1930-
let err = data.validate().unwrap_err();
1923+
}];
1924+
let err = validate_donut_charts(&data).unwrap_err();
19311925
assert!(err.message.contains("labels"));
19321926
}
19331927

19341928
#[test]
19351929
fn donut_rejects_empty_multiple() {
1936-
let data = DonutChartData::Multiple(vec![]);
1937-
assert!(data.validate().is_err());
1930+
let data: Vec<SingleDonutChart> = vec![];
1931+
assert!(validate_donut_charts(&data).is_err());
19381932
}
19391933

19401934
#[test]

0 commit comments

Comments
 (0)