Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -263,32 +264,51 @@
}

/**
* 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<Series> allSeries,
Map<String, AbstractSeries> seriesConfig) {
if (seriesConfig.isEmpty()) {
return;
}
for (var series : allSeries) {
if (!(series instanceof AbstractSeries as)
|| as.getName() == null) {
continue;
// Pre-scan: which template names have a matching data series?
var nameMatched = new HashSet<String>();
for (var s : allSeries) {
if (s instanceof AbstractSeries as
&& seriesConfig.containsKey(as.getName())) {
nameMatched.add(as.getName());
}
var template = seriesConfig.get(as.getName());
if (template == null) {
}

// 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) {

Check warning on line 287 in vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/chart/ChartRenderer.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Combine this loop with the one that starts on line 276.

See more on https://sonarcloud.io/project/issues?id=vaadin_flow-components&issues=AZ2aKkPlfvFglU1MyERH&open=AZ2aKkPlfvFglU1MyERH&pullRequest=9142
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,15 @@
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;
import com.vaadin.flow.component.charts.model.DataSeries;
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;

Expand Down Expand Up @@ -566,13 +568,196 @@ 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");
}

@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,
((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
// 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 = (AbstractSeries) series.get(1);
Assertions.assertEquals("Count", countSeries.getName());
Assertions.assertEquals(1, countSeries.getyAxis(),
"Positional match should apply yAxis from template");
Assertions.assertInstanceOf(PlotOptionsLine.class,
countSeries.getPlotOptions(),
"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(
Expand Down
Loading