Skip to content

Commit 83827cd

Browse files
tomivirkkiclaude
tomivirkki
and
claude
authored
fix: match config series to data by name, fall back to position (#9142)
## Summary - Series configuration templates (names, `plotOptions`, `yAxis`) are matched to data series by name first, with positional fallback for unmatched ones, so LLM-specified legend labels always take effect without swapping data under the wrong labels when SQL returns series in a different order than the config - Previously, `applySeriesConfig` only matched by name, which failed for unnamed multi-query series (e.g., candlestick + volume) and for `_SERIES`-named series where the config used different labels Before: <img width="1003" height="687" alt="Screenshot 2026-04-16 at 17 10 46" src="https://github.com/user-attachments/assets/63151d1c-9a18-47de-809f-312e41c2f241" /> After: <img width="1005" height="686" alt="Screenshot 2026-04-16 at 17 12 45" src="https://github.com/user-attachments/assets/3ab5ac09-cc8c-4aee-a8c0-fd8b104a5343" /> 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent e76794d commit 83827cd

2 files changed

Lines changed: 222 additions & 17 deletions

File tree

‎vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/chart/ChartRenderer.java‎

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import java.io.Serializable;
1919
import java.util.ArrayList;
20+
import java.util.HashSet;
2021
import java.util.LinkedHashMap;
2122
import java.util.LinkedHashSet;
2223
import java.util.List;
@@ -263,32 +264,51 @@ private static Map<String, AbstractSeries> extractSeriesConfig(
263264
}
264265

265266
/**
266-
* Applies previously extracted series configuration to the data series,
267-
* matching by name.
267+
* Applies previously extracted series configuration to the data series.
268+
* Matches by name first, then falls back to positional matching for
269+
* unmatched series — copying the template's name, plot options, and y-axis
270+
* binding.
268271
*/
269272
private static void applySeriesConfig(List<Series> allSeries,
270273
Map<String, AbstractSeries> seriesConfig) {
271-
if (seriesConfig.isEmpty()) {
272-
return;
273-
}
274-
for (var series : allSeries) {
275-
if (!(series instanceof AbstractSeries as)
276-
|| as.getName() == null) {
277-
continue;
274+
// Pre-scan: which template names have a matching data series?
275+
var nameMatched = new HashSet<String>();
276+
for (var s : allSeries) {
277+
if (s instanceof AbstractSeries as
278+
&& seriesConfig.containsKey(as.getName())) {
279+
nameMatched.add(as.getName());
278280
}
279-
var template = seriesConfig.get(as.getName());
280-
if (template == null) {
281+
}
282+
283+
// Templates without a name match feed the positional fallback.
284+
var positional = seriesConfig.values().stream()
285+
.filter(t -> !nameMatched.contains(t.getName())).iterator();
286+
287+
for (var s : allSeries) {
288+
if (!(s instanceof AbstractSeries as)) {
281289
continue;
282290
}
283-
if (template.getPlotOptions() != null) {
284-
as.setPlotOptions(template.getPlotOptions());
291+
var tpl = seriesConfig.get(as.getName());
292+
if (tpl == null && positional.hasNext()) {
293+
tpl = positional.next();
294+
as.setName(tpl.getName());
285295
}
286-
if (template.getyAxis() != null) {
287-
as.setyAxis(template.getyAxis());
296+
if (tpl != null) {
297+
applyTemplate(as, tpl);
288298
}
289299
}
290300
}
291301

302+
private static void applyTemplate(AbstractSeries target,
303+
AbstractSeries template) {
304+
if (template.getPlotOptions() != null) {
305+
target.setPlotOptions(template.getPlotOptions());
306+
}
307+
if (template.getyAxis() != null) {
308+
target.setyAxis(template.getyAxis());
309+
}
310+
}
311+
292312
/**
293313
* Checks whether any series has all non-null X values that look like epoch
294314
* millisecond timestamps (after year 2000). Checks per-series to handle

‎vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/chart/ChartRenderingTest.java‎

Lines changed: 187 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,15 @@
2929
import com.vaadin.flow.component.ai.provider.DatabaseProvider;
3030
import com.vaadin.flow.component.ai.provider.LLMProvider;
3131
import com.vaadin.flow.component.charts.Chart;
32+
import com.vaadin.flow.component.charts.model.AbstractSeries;
3233
import com.vaadin.flow.component.charts.model.AxisType;
3334
import com.vaadin.flow.component.charts.model.ChartType;
3435
import com.vaadin.flow.component.charts.model.Configuration;
3536
import com.vaadin.flow.component.charts.model.DataSeries;
3637
import com.vaadin.flow.component.charts.model.DataSeriesItem;
3738
import com.vaadin.flow.component.charts.model.OhlcItem;
3839
import com.vaadin.flow.component.charts.model.PlotOptionsFlags;
40+
import com.vaadin.flow.component.charts.model.PlotOptionsLine;
3941
import com.vaadin.flow.component.charts.util.ChartSerialization;
4042
import com.vaadin.tests.MockUIExtension;
4143

@@ -566,13 +568,196 @@ void perSeriesPlotOptions_appliedToSingleUnnamedSeries() {
566568
Assertions.assertEquals(1, series.size());
567569
// Series should be named from title AND have plotOptions applied
568570
Assertions.assertEquals("Revenue", series.get(0).getName());
569-
var plotOptions = ((com.vaadin.flow.component.charts.model.AbstractSeries) series
570-
.get(0)).getPlotOptions();
571+
var plotOptions = ((AbstractSeries) series.get(0)).getPlotOptions();
571572
Assertions.assertNotNull(plotOptions,
572573
"Per-series plotOptions should be applied to "
573574
+ "single unnamed series after title-based naming");
574575
}
575576

577+
@Test
578+
void multiQueryUnnamedSeriesReceiveNamesFromConfig() {
579+
// When two separate queries produce unnamed series (e.g.
580+
// candlestick OHLC + volume) and the LLM config provides a
581+
// series array with names, the names should be applied
582+
// positionally to the unnamed data series.
583+
int[] callCount = { 0 };
584+
controller.setDataConverter(data -> {
585+
callCount[0]++;
586+
if (callCount[0] == 1) {
587+
DataSeries ohlcSeries = new DataSeries();
588+
ohlcSeries.add(new OhlcItem(1704067200000L, 142.5, 148.2,
589+
141.0, 147.8));
590+
return List.of(ohlcSeries);
591+
} else {
592+
DataSeries volumeSeries = new DataSeries();
593+
volumeSeries.add(new DataSeriesItem(1704067200000L, 52000));
594+
return List.of(volumeSeries);
595+
}
596+
});
597+
598+
updateConfiguration("""
599+
{"chart":{"type":"candlestick"},
600+
"yAxis":[{"title":{"text":"Price"}},
601+
{"title":{"text":"Volume"},"opposite":true}],
602+
"series":[{"name":"Prices","type":"candlestick"},
603+
{"name":"Vol","type":"column","yAxis":1}]}
604+
""");
605+
updateData("SELECT 1", "SELECT 2");
606+
controller.onRequestCompleted();
607+
608+
var series = chart.getConfiguration().getSeries();
609+
Assertions.assertEquals(2, series.size());
610+
Assertions.assertEquals("Prices", series.get(0).getName(),
611+
"First unnamed series should receive name from config");
612+
Assertions.assertEquals("Vol", series.get(1).getName(),
613+
"Second unnamed series should receive name from config");
614+
}
615+
616+
@Test
617+
void configSeriesNamesOverrideSeriesColumnNames() {
618+
// When _SERIES column produces named series but the config
619+
// also provides explicit series names, the config names
620+
// should win (the user asked for specific legend labels).
621+
databaseProvider.results = List.of(
622+
row(ColumnNames.SERIES, "North", "category", "Jan", "value",
623+
45000),
624+
row(ColumnNames.SERIES, "South", "category", "Jan", "value",
625+
38000));
626+
627+
updateConfiguration("""
628+
{"chart":{"type":"column"},
629+
"series":[{"name":"Revenue","yAxis":0},
630+
{"name":"Costs","yAxis":1}]}
631+
""");
632+
updateData("SELECT s, c, v FROM t");
633+
controller.onRequestCompleted();
634+
635+
var series = chart.getConfiguration().getSeries();
636+
Assertions.assertEquals(2, series.size());
637+
Assertions.assertEquals("Revenue", series.get(0).getName(),
638+
"Config name should override _SERIES name");
639+
Assertions.assertEquals("Costs", series.get(1).getName(),
640+
"Config name should override _SERIES name");
641+
}
642+
643+
@Test
644+
void nameMatchedSeriesNotOverwrittenByPositionalFallback() {
645+
// When one data series matches a config template by name and
646+
// another doesn't, the name-matched series must not be
647+
// re-processed by positional fallback. The name-matched
648+
// series ("Revenue") is deliberately second in data order so
649+
// that positional fallback — if it ignored matched tracking —
650+
// would assign it the wrong template.
651+
databaseProvider.results = List.of(
652+
row(ColumnNames.SERIES, "Other", "category", "Jan", "value",
653+
38000),
654+
row(ColumnNames.SERIES, "Revenue", "category", "Jan",
655+
"value", 45000));
656+
657+
updateConfiguration("{\"chart\":{\"type\":\"column\"},"
658+
+ "\"series\":[" + "{\"name\":\"Revenue\",\"yAxis\":0},"
659+
+ "{\"name\":\"Costs\",\"yAxis\":1}" + "]}");
660+
updateData("SELECT s, c, v FROM t");
661+
controller.onRequestCompleted();
662+
663+
var series = chart.getConfiguration().getSeries();
664+
Assertions.assertEquals(2, series.size());
665+
// "Other" did not match — positional fallback renames to "Costs"
666+
Assertions.assertEquals("Costs", series.get(0).getName());
667+
Assertions.assertEquals(1,
668+
((AbstractSeries) series.get(0)).getyAxis(),
669+
"Positionally matched series should get template yAxis");
670+
// "Revenue" matched by name — must keep its name and yAxis
671+
Assertions.assertEquals("Revenue", series.get(1).getName());
672+
Assertions.assertEquals(0,
673+
((AbstractSeries) series.get(1)).getyAxis(),
674+
"Name-matched series should keep its yAxis");
675+
}
676+
677+
@Test
678+
void positionalMatchAppliesPlotOptionsAndYAxis() {
679+
// Positional fallback must apply plotOptions and yAxis from
680+
// the config template, not just the name.
681+
int[] callCount = { 0 };
682+
controller.setDataConverter(data -> {
683+
callCount[0]++;
684+
if (callCount[0] == 1) {
685+
DataSeries s = new DataSeries();
686+
s.add(new DataSeriesItem("Jan", 100));
687+
return List.of(s);
688+
} else {
689+
DataSeries s = new DataSeries();
690+
s.add(new DataSeriesItem("Jan", 200));
691+
return List.of(s);
692+
}
693+
});
694+
695+
updateConfiguration("""
696+
{"chart":{"type":"column"},
697+
"series":[{"name":"Revenue","type":"column","yAxis":0},
698+
{"name":"Count","type":"line","yAxis":1}]}
699+
""");
700+
updateData("SELECT 1", "SELECT 2");
701+
controller.onRequestCompleted();
702+
703+
var series = chart.getConfiguration().getSeries();
704+
Assertions.assertEquals(2, series.size());
705+
var countSeries = (AbstractSeries) series.get(1);
706+
Assertions.assertEquals("Count", countSeries.getName());
707+
Assertions.assertEquals(1, countSeries.getyAxis(),
708+
"Positional match should apply yAxis from template");
709+
Assertions.assertInstanceOf(PlotOptionsLine.class,
710+
countSeries.getPlotOptions(),
711+
"Positional match should apply plotOptions from template");
712+
}
713+
714+
@Test
715+
void dataOrderDifferentFromConfigOrderPreservesDataLabels() {
716+
// The LLM writes the config series[] array without any
717+
// guarantee that the SQL data will come back in that order
718+
// (GROUP BY without ORDER BY, DB-side hash ordering, etc.).
719+
// When data order differs from config order, matching by
720+
// position renames data series to the WRONG labels —
721+
// effectively swapping data points under their legend
722+
// labels. Matching by name (the prior behavior) avoids this.
723+
controller.setDataConverter(data -> {
724+
// Simulate DB returning "South" before "North"
725+
DataSeries south = new DataSeries("South");
726+
south.add(new DataSeriesItem("Jan", 38000));
727+
DataSeries north = new DataSeries("North");
728+
north.add(new DataSeriesItem("Jan", 45000));
729+
return List.of(south, north);
730+
});
731+
732+
updateConfiguration("""
733+
{"chart":{"type":"column"},
734+
"yAxis":[{"title":{"text":"Primary"}},
735+
{"title":{"text":"Secondary"},"opposite":true}],
736+
"series":[{"name":"North","yAxis":0},
737+
{"name":"South","yAxis":1}]}
738+
""");
739+
updateData("SELECT s, c, v FROM t");
740+
controller.onRequestCompleted();
741+
742+
var series = chart.getConfiguration().getSeries();
743+
Assertions.assertEquals(2, series.size());
744+
// The first data series is South (value 38000); its label
745+
// must remain "South", not be overwritten to "North".
746+
Assertions.assertEquals("South", series.get(0).getName(),
747+
"Data series labels must not be swapped when data "
748+
+ "order differs from config order");
749+
Assertions.assertEquals("North", series.get(1).getName());
750+
// Similarly the yAxis assignment should follow the name,
751+
// so that "South" sits on the Secondary axis (yAxis=1).
752+
Assertions.assertEquals(1,
753+
((AbstractSeries) series.get(0)).getyAxis(),
754+
"South should end up on Secondary y-axis (yAxis=1), "
755+
+ "not Primary");
756+
Assertions.assertEquals(0,
757+
((AbstractSeries) series.get(1)).getyAxis(),
758+
"North should end up on Primary y-axis (yAxis=0)");
759+
}
760+
576761
@Test
577762
void multipleSeriesWithSeriesColumnKeepsOriginalNames() {
578763
databaseProvider.results = List.of(

0 commit comments

Comments
 (0)