From 91e7dc776f29cba5c96fa20607ce853ce4208946 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Thu, 16 Apr 2026 16:09:16 +0300 Subject: [PATCH 1/5] fix: apply config series names to unnamed and mismatched data series Co-Authored-By: Claude Opus 4.6 (1M context) --- .../component/ai/chart/ChartRenderer.java | 53 ++++--- .../ai/chart/ChartRenderingTest.java | 140 ++++++++++++++++++ 2 files changed, 175 insertions(+), 18 deletions(-) diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/chart/ChartRenderer.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/chart/ChartRenderer.java index 398421f848e..a35f6b1bb49 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/chart/ChartRenderer.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/chart/ChartRenderer.java @@ -263,30 +263,47 @@ private static Map extractSeriesConfig( } /** - * Applies previously extracted series configuration to the data series, - * matching by name. + * Applies previously extracted series configuration to the data series. + * Matches by name first, then falls back to positional matching for + * unmatched series — copying the template's name, plot options, and + * y-axis binding. */ private static void applySeriesConfig(List allSeries, Map seriesConfig) { - if (seriesConfig.isEmpty()) { - return; - } + // Pass 1: match by name + var matched = new java.util.HashSet(); for (var series : allSeries) { - if (!(series instanceof AbstractSeries as) - || as.getName() == null) { - continue; - } - var template = seriesConfig.get(as.getName()); - if (template == null) { - continue; - } - if (template.getPlotOptions() != null) { - as.setPlotOptions(template.getPlotOptions()); - } - if (template.getyAxis() != null) { - as.setyAxis(template.getyAxis()); + if (series instanceof AbstractSeries as + && as.getName() != null) { + var tpl = seriesConfig.get(as.getName()); + if (tpl != null) { + applyTemplate(as, tpl); + matched.add(as.getName()); + } } } + + // Pass 2: positional fallback for unmatched series + var templates = seriesConfig.values().stream() + .filter(t -> !matched.contains(t.getName())).toList(); + var data = allSeries.stream() + .filter(AbstractSeries.class::isInstance) + .map(AbstractSeries.class::cast) + .filter(as -> !matched.contains(as.getName())).toList(); + for (int i = 0; i < Math.min(data.size(), templates.size()); i++) { + data.get(i).setName(templates.get(i).getName()); + applyTemplate(data.get(i), templates.get(i)); + } + } + + private static void applyTemplate(AbstractSeries target, + AbstractSeries template) { + if (template.getPlotOptions() != null) { + target.setPlotOptions(template.getPlotOptions()); + } + if (template.getyAxis() != null) { + target.setyAxis(template.getyAxis()); + } } /** diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/chart/ChartRenderingTest.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/chart/ChartRenderingTest.java index c3ecb386c22..41bada7d145 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/chart/ChartRenderingTest.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/chart/ChartRenderingTest.java @@ -573,6 +573,146 @@ void perSeriesPlotOptions_appliedToSingleUnnamedSeries() { + "single unnamed series after title-based naming"); } + @Test + void multiQueryUnnamedSeriesReceiveNamesFromConfig() { + // When two separate queries produce unnamed series (e.g. + // candlestick OHLC + volume) and the LLM config provides a + // series array with names, the names should be applied + // positionally to the unnamed data series. + int[] callCount = { 0 }; + controller.setDataConverter(data -> { + callCount[0]++; + if (callCount[0] == 1) { + DataSeries ohlcSeries = new DataSeries(); + ohlcSeries.add(new OhlcItem(1704067200000L, 142.5, 148.2, + 141.0, 147.8)); + return List.of(ohlcSeries); + } else { + DataSeries volumeSeries = new DataSeries(); + volumeSeries.add(new DataSeriesItem(1704067200000L, 52000)); + return List.of(volumeSeries); + } + }); + + updateConfiguration("{\"chart\":{\"type\":\"candlestick\"}," + + "\"yAxis\":[{\"title\":{\"text\":\"Price\"}}," + + "{\"title\":{\"text\":\"Volume\"},\"opposite\":true}]," + + "\"series\":[" + + "{\"name\":\"Prices\",\"type\":\"candlestick\"}," + + "{\"name\":\"Vol\",\"type\":\"column\",\"yAxis\":1}" + + "]}"); + updateData("SELECT 1", "SELECT 2"); + controller.onRequestCompleted(); + + var series = chart.getConfiguration().getSeries(); + Assertions.assertEquals(2, series.size()); + Assertions.assertEquals("Prices", series.get(0).getName(), + "First unnamed series should receive name from config"); + Assertions.assertEquals("Vol", series.get(1).getName(), + "Second unnamed series should receive name from config"); + } + + @Test + void configSeriesNamesOverrideSeriesColumnNames() { + // When _SERIES column produces named series but the config + // also provides explicit series names, the config names + // should win (the user asked for specific legend labels). + databaseProvider.results = List.of( + row(ColumnNames.SERIES, "North", "category", "Jan", "value", + 45000), + row(ColumnNames.SERIES, "South", "category", "Jan", "value", + 38000)); + + updateConfiguration("{\"chart\":{\"type\":\"column\"}," + + "\"series\":[" + "{\"name\":\"Revenue\",\"yAxis\":0}," + + "{\"name\":\"Costs\",\"yAxis\":1}" + "]}"); + updateData("SELECT s, c, v FROM t"); + controller.onRequestCompleted(); + + var series = chart.getConfiguration().getSeries(); + Assertions.assertEquals(2, series.size()); + Assertions.assertEquals("Revenue", series.get(0).getName(), + "Config name should override _SERIES name"); + Assertions.assertEquals("Costs", series.get(1).getName(), + "Config name should override _SERIES name"); + } + + @Test + void nameMatchedSeriesNotOverwrittenByPositionalFallback() { + // When one data series matches a config template by name and + // another doesn't, the name-matched series must not be + // re-processed by positional fallback. The name-matched + // series ("Revenue") is deliberately second in data order so + // that positional fallback — if it ignored matched tracking — + // would assign it the wrong template. + databaseProvider.results = List.of( + row(ColumnNames.SERIES, "Other", "category", "Jan", + "value", 38000), + row(ColumnNames.SERIES, "Revenue", "category", "Jan", + "value", 45000)); + + updateConfiguration("{\"chart\":{\"type\":\"column\"}," + + "\"series\":[" + + "{\"name\":\"Revenue\",\"yAxis\":0}," + + "{\"name\":\"Costs\",\"yAxis\":1}" + "]}"); + updateData("SELECT s, c, v FROM t"); + controller.onRequestCompleted(); + + var series = chart.getConfiguration().getSeries(); + Assertions.assertEquals(2, series.size()); + // "Other" did not match — positional fallback renames to "Costs" + Assertions.assertEquals("Costs", series.get(0).getName()); + Assertions.assertEquals(1, + ((com.vaadin.flow.component.charts.model.AbstractSeries) series + .get(0)).getyAxis(), + "Positionally matched series should get template yAxis"); + // "Revenue" matched by name — must keep its name and yAxis + Assertions.assertEquals("Revenue", series.get(1).getName()); + Assertions.assertEquals(0, + ((com.vaadin.flow.component.charts.model.AbstractSeries) series + .get(1)).getyAxis(), + "Name-matched series should keep its yAxis"); + } + + @Test + void positionalMatchAppliesPlotOptionsAndYAxis() { + // Positional fallback must apply plotOptions and yAxis from + // the config template, not just the name. + int[] callCount = { 0 }; + controller.setDataConverter(data -> { + callCount[0]++; + if (callCount[0] == 1) { + DataSeries s = new DataSeries(); + s.add(new DataSeriesItem("Jan", 100)); + return List.of(s); + } else { + DataSeries s = new DataSeries(); + s.add(new DataSeriesItem("Jan", 200)); + return List.of(s); + } + }); + + updateConfiguration("{\"chart\":{\"type\":\"column\"}," + + "\"series\":[" + + "{\"name\":\"Revenue\",\"type\":\"column\",\"yAxis\":0}," + + "{\"name\":\"Count\",\"type\":\"line\",\"yAxis\":1}" + + "]}"); + updateData("SELECT 1", "SELECT 2"); + controller.onRequestCompleted(); + + var series = chart.getConfiguration().getSeries(); + Assertions.assertEquals(2, series.size()); + var countSeries = (com.vaadin.flow.component.charts.model.AbstractSeries) series + .get(1); + Assertions.assertEquals("Count", countSeries.getName()); + Assertions.assertEquals(1, countSeries.getyAxis(), + "Positional match should apply yAxis from template"); + Assertions.assertInstanceOf( + com.vaadin.flow.component.charts.model.PlotOptionsLine.class, + countSeries.getPlotOptions(), + "Positional match should apply plotOptions from template"); + } + @Test void multipleSeriesWithSeriesColumnKeepsOriginalNames() { databaseProvider.results = List.of( From eb8dac2cf6e5999e6127ae4b34569e15e35591aa Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Thu, 16 Apr 2026 16:30:32 +0300 Subject: [PATCH 2/5] Cleanup --- .../component/ai/chart/ChartRenderer.java | 43 ++++++++++--------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/chart/ChartRenderer.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/chart/ChartRenderer.java index a35f6b1bb49..1442107bfa0 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/chart/ChartRenderer.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/chart/ChartRenderer.java @@ -17,6 +17,7 @@ import java.io.Serializable; import java.util.ArrayList; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -270,29 +271,31 @@ private static Map extractSeriesConfig( */ private static void applySeriesConfig(List allSeries, Map seriesConfig) { - // Pass 1: match by name - var matched = new java.util.HashSet(); - for (var series : allSeries) { - if (series instanceof AbstractSeries as - && as.getName() != null) { - var tpl = seriesConfig.get(as.getName()); - if (tpl != null) { - applyTemplate(as, tpl); - matched.add(as.getName()); - } + // Pre-scan: which template names have a matching data series? + var nameMatched = new HashSet(); + for (var s : allSeries) { + if (s instanceof AbstractSeries as + && seriesConfig.containsKey(as.getName())) { + nameMatched.add(as.getName()); } } - // Pass 2: positional fallback for unmatched series - var templates = seriesConfig.values().stream() - .filter(t -> !matched.contains(t.getName())).toList(); - var data = allSeries.stream() - .filter(AbstractSeries.class::isInstance) - .map(AbstractSeries.class::cast) - .filter(as -> !matched.contains(as.getName())).toList(); - for (int i = 0; i < Math.min(data.size(), templates.size()); i++) { - data.get(i).setName(templates.get(i).getName()); - applyTemplate(data.get(i), templates.get(i)); + // Templates without a name match feed the positional fallback. + var positional = seriesConfig.values().stream() + .filter(t -> !nameMatched.contains(t.getName())).iterator(); + + for (var s : allSeries) { + if (!(s instanceof AbstractSeries as)) { + continue; + } + var tpl = seriesConfig.get(as.getName()); + if (tpl == null && positional.hasNext()) { + tpl = positional.next(); + as.setName(tpl.getName()); + } + if (tpl != null) { + applyTemplate(as, tpl); + } } } From cd48ac06f18d52f55a08b94353dc1c7a80c9f117 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Thu, 16 Apr 2026 16:47:14 +0300 Subject: [PATCH 3/5] refactor: simplify applySeriesConfig to pure positional matching Co-Authored-By: Claude Opus 4.6 (1M context) --- .../component/ai/chart/ChartRenderer.java | 57 +++++++------------ .../ai/chart/ChartRenderingTest.java | 37 ------------ 2 files changed, 19 insertions(+), 75 deletions(-) diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/chart/ChartRenderer.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/chart/ChartRenderer.java index 1442107bfa0..b77749193a7 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/chart/ChartRenderer.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/chart/ChartRenderer.java @@ -17,7 +17,6 @@ import java.io.Serializable; import java.util.ArrayList; -import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -264,48 +263,30 @@ private static Map extractSeriesConfig( } /** - * Applies previously extracted series configuration to the data series. - * Matches by name first, then falls back to positional matching for - * unmatched series — copying the template's name, plot options, and - * y-axis binding. + * Applies series configuration templates to data series by position. + * Templates and data series are assumed to be in the same order (both + * produced by the LLM in a single request). If a template has a name, it + * overrides the data series name so the legend shows the LLM-specified + * label. */ - private static void applySeriesConfig(List allSeries, - Map seriesConfig) { - // Pre-scan: which template names have a matching data series? - var nameMatched = new HashSet(); - for (var s : allSeries) { - if (s instanceof AbstractSeries as - && seriesConfig.containsKey(as.getName())) { - nameMatched.add(as.getName()); - } - } - - // Templates without a name match feed the positional fallback. - var positional = seriesConfig.values().stream() - .filter(t -> !nameMatched.contains(t.getName())).iterator(); - - for (var s : allSeries) { - if (!(s instanceof AbstractSeries as)) { + private static void applySeriesConfig(List dataSeries, + Map templates) { + var templateList = new ArrayList<>(templates.values()); + for (int i = 0; i < Math.min(dataSeries.size(), + templateList.size()); i++) { + if (!(dataSeries.get(i) instanceof AbstractSeries as)) { continue; } - var tpl = seriesConfig.get(as.getName()); - if (tpl == null && positional.hasNext()) { - tpl = positional.next(); - as.setName(tpl.getName()); + var template = templateList.get(i); + if (template.getName() != null) { + as.setName(template.getName()); } - if (tpl != null) { - applyTemplate(as, tpl); + if (template.getPlotOptions() != null) { + as.setPlotOptions(template.getPlotOptions()); + } + if (template.getyAxis() != null) { + as.setyAxis(template.getyAxis()); } - } - } - - private static void applyTemplate(AbstractSeries target, - AbstractSeries template) { - if (template.getPlotOptions() != null) { - target.setPlotOptions(template.getPlotOptions()); - } - if (template.getyAxis() != null) { - target.setyAxis(template.getyAxis()); } } diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/chart/ChartRenderingTest.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/chart/ChartRenderingTest.java index 41bada7d145..742acda7f25 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/chart/ChartRenderingTest.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/chart/ChartRenderingTest.java @@ -637,43 +637,6 @@ void configSeriesNamesOverrideSeriesColumnNames() { "Config name should override _SERIES name"); } - @Test - void nameMatchedSeriesNotOverwrittenByPositionalFallback() { - // When one data series matches a config template by name and - // another doesn't, the name-matched series must not be - // re-processed by positional fallback. The name-matched - // series ("Revenue") is deliberately second in data order so - // that positional fallback — if it ignored matched tracking — - // would assign it the wrong template. - databaseProvider.results = List.of( - row(ColumnNames.SERIES, "Other", "category", "Jan", - "value", 38000), - row(ColumnNames.SERIES, "Revenue", "category", "Jan", - "value", 45000)); - - updateConfiguration("{\"chart\":{\"type\":\"column\"}," - + "\"series\":[" - + "{\"name\":\"Revenue\",\"yAxis\":0}," - + "{\"name\":\"Costs\",\"yAxis\":1}" + "]}"); - updateData("SELECT s, c, v FROM t"); - controller.onRequestCompleted(); - - var series = chart.getConfiguration().getSeries(); - Assertions.assertEquals(2, series.size()); - // "Other" did not match — positional fallback renames to "Costs" - Assertions.assertEquals("Costs", series.get(0).getName()); - Assertions.assertEquals(1, - ((com.vaadin.flow.component.charts.model.AbstractSeries) series - .get(0)).getyAxis(), - "Positionally matched series should get template yAxis"); - // "Revenue" matched by name — must keep its name and yAxis - Assertions.assertEquals("Revenue", series.get(1).getName()); - Assertions.assertEquals(0, - ((com.vaadin.flow.component.charts.model.AbstractSeries) series - .get(1)).getyAxis(), - "Name-matched series should keep its yAxis"); - } - @Test void positionalMatchAppliesPlotOptionsAndYAxis() { // Positional fallback must apply plotOptions and yAxis from From 5df07fb27e0e920975a14373a227c4380e2217ae Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Thu, 16 Apr 2026 17:00:56 +0300 Subject: [PATCH 4/5] Tests cleanup --- .../ai/chart/ChartRenderingTest.java | 43 ++++++++++--------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/chart/ChartRenderingTest.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/chart/ChartRenderingTest.java index 742acda7f25..f1ddf58c5a2 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/chart/ChartRenderingTest.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/chart/ChartRenderingTest.java @@ -29,6 +29,7 @@ import com.vaadin.flow.component.ai.provider.DatabaseProvider; import com.vaadin.flow.component.ai.provider.LLMProvider; import com.vaadin.flow.component.charts.Chart; +import com.vaadin.flow.component.charts.model.AbstractSeries; import com.vaadin.flow.component.charts.model.AxisType; import com.vaadin.flow.component.charts.model.ChartType; import com.vaadin.flow.component.charts.model.Configuration; @@ -36,6 +37,7 @@ import com.vaadin.flow.component.charts.model.DataSeriesItem; import com.vaadin.flow.component.charts.model.OhlcItem; import com.vaadin.flow.component.charts.model.PlotOptionsFlags; +import com.vaadin.flow.component.charts.model.PlotOptionsLine; import com.vaadin.flow.component.charts.util.ChartSerialization; import com.vaadin.tests.MockUIExtension; @@ -566,8 +568,7 @@ void perSeriesPlotOptions_appliedToSingleUnnamedSeries() { Assertions.assertEquals(1, series.size()); // Series should be named from title AND have plotOptions applied Assertions.assertEquals("Revenue", series.get(0).getName()); - var plotOptions = ((com.vaadin.flow.component.charts.model.AbstractSeries) series - .get(0)).getPlotOptions(); + var plotOptions = ((AbstractSeries) series.get(0)).getPlotOptions(); Assertions.assertNotNull(plotOptions, "Per-series plotOptions should be applied to " + "single unnamed series after title-based naming"); @@ -594,13 +595,13 @@ void multiQueryUnnamedSeriesReceiveNamesFromConfig() { } }); - updateConfiguration("{\"chart\":{\"type\":\"candlestick\"}," - + "\"yAxis\":[{\"title\":{\"text\":\"Price\"}}," - + "{\"title\":{\"text\":\"Volume\"},\"opposite\":true}]," - + "\"series\":[" - + "{\"name\":\"Prices\",\"type\":\"candlestick\"}," - + "{\"name\":\"Vol\",\"type\":\"column\",\"yAxis\":1}" - + "]}"); + updateConfiguration(""" + {"chart":{"type":"candlestick"}, + "yAxis":[{"title":{"text":"Price"}}, + {"title":{"text":"Volume"},"opposite":true}], + "series":[{"name":"Prices","type":"candlestick"}, + {"name":"Vol","type":"column","yAxis":1}]} + """); updateData("SELECT 1", "SELECT 2"); controller.onRequestCompleted(); @@ -623,9 +624,11 @@ void configSeriesNamesOverrideSeriesColumnNames() { row(ColumnNames.SERIES, "South", "category", "Jan", "value", 38000)); - updateConfiguration("{\"chart\":{\"type\":\"column\"}," - + "\"series\":[" + "{\"name\":\"Revenue\",\"yAxis\":0}," - + "{\"name\":\"Costs\",\"yAxis\":1}" + "]}"); + updateConfiguration(""" + {"chart":{"type":"column"}, + "series":[{"name":"Revenue","yAxis":0}, + {"name":"Costs","yAxis":1}]} + """); updateData("SELECT s, c, v FROM t"); controller.onRequestCompleted(); @@ -655,23 +658,21 @@ void positionalMatchAppliesPlotOptionsAndYAxis() { } }); - updateConfiguration("{\"chart\":{\"type\":\"column\"}," - + "\"series\":[" - + "{\"name\":\"Revenue\",\"type\":\"column\",\"yAxis\":0}," - + "{\"name\":\"Count\",\"type\":\"line\",\"yAxis\":1}" - + "]}"); + updateConfiguration(""" + {"chart":{"type":"column"}, + "series":[{"name":"Revenue","type":"column","yAxis":0}, + {"name":"Count","type":"line","yAxis":1}]} + """); updateData("SELECT 1", "SELECT 2"); controller.onRequestCompleted(); var series = chart.getConfiguration().getSeries(); Assertions.assertEquals(2, series.size()); - var countSeries = (com.vaadin.flow.component.charts.model.AbstractSeries) series - .get(1); + var countSeries = (AbstractSeries) series.get(1); Assertions.assertEquals("Count", countSeries.getName()); Assertions.assertEquals(1, countSeries.getyAxis(), "Positional match should apply yAxis from template"); - Assertions.assertInstanceOf( - com.vaadin.flow.component.charts.model.PlotOptionsLine.class, + Assertions.assertInstanceOf(PlotOptionsLine.class, countSeries.getPlotOptions(), "Positional match should apply plotOptions from template"); } From 9d06641d9972c317ec3d3f9e61dd98d0f7b1cc61 Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Fri, 17 Apr 2026 09:32:48 +0300 Subject: [PATCH 5/5] revert: restore name-based series matching with positional fallback Co-Authored-By: Claude Opus 4.7 (1M context) --- .../component/ai/chart/ChartRenderer.java | 57 ++++++++----- .../ai/chart/ChartRenderingTest.java | 81 +++++++++++++++++++ 2 files changed, 119 insertions(+), 19 deletions(-) diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/chart/ChartRenderer.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/chart/ChartRenderer.java index b77749193a7..e8ea13e20d1 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/chart/ChartRenderer.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/chart/ChartRenderer.java @@ -17,6 +17,7 @@ import java.io.Serializable; import java.util.ArrayList; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -263,33 +264,51 @@ private static Map extractSeriesConfig( } /** - * Applies series configuration templates to data series by position. - * Templates and data series are assumed to be in the same order (both - * produced by the LLM in a single request). If a template has a name, it - * overrides the data series name so the legend shows the LLM-specified - * label. + * Applies previously extracted series configuration to the data series. + * Matches by name first, then falls back to positional matching for + * unmatched series — copying the template's name, plot options, and y-axis + * binding. */ - private static void applySeriesConfig(List dataSeries, - Map templates) { - var templateList = new ArrayList<>(templates.values()); - for (int i = 0; i < Math.min(dataSeries.size(), - templateList.size()); i++) { - if (!(dataSeries.get(i) instanceof AbstractSeries as)) { - continue; + private static void applySeriesConfig(List allSeries, + Map seriesConfig) { + // Pre-scan: which template names have a matching data series? + var nameMatched = new HashSet(); + for (var s : allSeries) { + if (s instanceof AbstractSeries as + && seriesConfig.containsKey(as.getName())) { + nameMatched.add(as.getName()); } - var template = templateList.get(i); - if (template.getName() != null) { - as.setName(template.getName()); + } + + // Templates without a name match feed the positional fallback. + var positional = seriesConfig.values().stream() + .filter(t -> !nameMatched.contains(t.getName())).iterator(); + + for (var s : allSeries) { + if (!(s instanceof AbstractSeries as)) { + continue; } - if (template.getPlotOptions() != null) { - as.setPlotOptions(template.getPlotOptions()); + var tpl = seriesConfig.get(as.getName()); + if (tpl == null && positional.hasNext()) { + tpl = positional.next(); + as.setName(tpl.getName()); } - if (template.getyAxis() != null) { - as.setyAxis(template.getyAxis()); + if (tpl != null) { + applyTemplate(as, tpl); } } } + private static void applyTemplate(AbstractSeries target, + AbstractSeries template) { + if (template.getPlotOptions() != null) { + target.setPlotOptions(template.getPlotOptions()); + } + if (template.getyAxis() != null) { + target.setyAxis(template.getyAxis()); + } + } + /** * Checks whether any series has all non-null X values that look like epoch * millisecond timestamps (after year 2000). Checks per-series to handle diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/chart/ChartRenderingTest.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/chart/ChartRenderingTest.java index f1ddf58c5a2..26405795f69 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/chart/ChartRenderingTest.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/chart/ChartRenderingTest.java @@ -640,6 +640,40 @@ void configSeriesNamesOverrideSeriesColumnNames() { "Config name should override _SERIES name"); } + @Test + void nameMatchedSeriesNotOverwrittenByPositionalFallback() { + // When one data series matches a config template by name and + // another doesn't, the name-matched series must not be + // re-processed by positional fallback. The name-matched + // series ("Revenue") is deliberately second in data order so + // that positional fallback — if it ignored matched tracking — + // would assign it the wrong template. + databaseProvider.results = List.of( + row(ColumnNames.SERIES, "Other", "category", "Jan", "value", + 38000), + row(ColumnNames.SERIES, "Revenue", "category", "Jan", + "value", 45000)); + + updateConfiguration("{\"chart\":{\"type\":\"column\"}," + + "\"series\":[" + "{\"name\":\"Revenue\",\"yAxis\":0}," + + "{\"name\":\"Costs\",\"yAxis\":1}" + "]}"); + updateData("SELECT s, c, v FROM t"); + controller.onRequestCompleted(); + + var series = chart.getConfiguration().getSeries(); + Assertions.assertEquals(2, series.size()); + // "Other" did not match — positional fallback renames to "Costs" + Assertions.assertEquals("Costs", series.get(0).getName()); + Assertions.assertEquals(1, + ((AbstractSeries) series.get(0)).getyAxis(), + "Positionally matched series should get template yAxis"); + // "Revenue" matched by name — must keep its name and yAxis + Assertions.assertEquals("Revenue", series.get(1).getName()); + Assertions.assertEquals(0, + ((AbstractSeries) series.get(1)).getyAxis(), + "Name-matched series should keep its yAxis"); + } + @Test void positionalMatchAppliesPlotOptionsAndYAxis() { // Positional fallback must apply plotOptions and yAxis from @@ -677,6 +711,53 @@ void positionalMatchAppliesPlotOptionsAndYAxis() { "Positional match should apply plotOptions from template"); } + @Test + void dataOrderDifferentFromConfigOrderPreservesDataLabels() { + // The LLM writes the config series[] array without any + // guarantee that the SQL data will come back in that order + // (GROUP BY without ORDER BY, DB-side hash ordering, etc.). + // When data order differs from config order, matching by + // position renames data series to the WRONG labels — + // effectively swapping data points under their legend + // labels. Matching by name (the prior behavior) avoids this. + controller.setDataConverter(data -> { + // Simulate DB returning "South" before "North" + DataSeries south = new DataSeries("South"); + south.add(new DataSeriesItem("Jan", 38000)); + DataSeries north = new DataSeries("North"); + north.add(new DataSeriesItem("Jan", 45000)); + return List.of(south, north); + }); + + updateConfiguration(""" + {"chart":{"type":"column"}, + "yAxis":[{"title":{"text":"Primary"}}, + {"title":{"text":"Secondary"},"opposite":true}], + "series":[{"name":"North","yAxis":0}, + {"name":"South","yAxis":1}]} + """); + updateData("SELECT s, c, v FROM t"); + controller.onRequestCompleted(); + + var series = chart.getConfiguration().getSeries(); + Assertions.assertEquals(2, series.size()); + // The first data series is South (value 38000); its label + // must remain "South", not be overwritten to "North". + Assertions.assertEquals("South", series.get(0).getName(), + "Data series labels must not be swapped when data " + + "order differs from config order"); + Assertions.assertEquals("North", series.get(1).getName()); + // Similarly the yAxis assignment should follow the name, + // so that "South" sits on the Secondary axis (yAxis=1). + Assertions.assertEquals(1, + ((AbstractSeries) series.get(0)).getyAxis(), + "South should end up on Secondary y-axis (yAxis=1), " + + "not Primary"); + Assertions.assertEquals(0, + ((AbstractSeries) series.get(1)).getyAxis(), + "North should end up on Primary y-axis (yAxis=0)"); + } + @Test void multipleSeriesWithSeriesColumnKeepsOriginalNames() { databaseProvider.results = List.of(