From 83a6a96aed393bb45ac3d6da55dea12257343add Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Thu, 18 Jun 2026 14:14:23 +0300 Subject: [PATCH 1/4] feat: add item label generator to value options --- .../component/ai/form/FormAIController.java | 104 +++++++--- .../component/ai/form/FormFieldHints.java | 30 ++- .../component/ai/form/FormFieldSchema.java | 31 ++- .../flow/component/ai/form/ValueOptions.java | 95 +++++++--- .../component/ai/form/FillFormToolTest.java | 43 +++-- .../ai/form/FormAIControllerTest.java | 72 +++++-- .../component/ai/form/FormStateToolTest.java | 178 +++++++++++++++++- .../ai/form/QueryFieldOptionsToolTest.java | 59 +++++- 8 files changed, 504 insertions(+), 108 deletions(-) diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormAIController.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormAIController.java index 338f78a1866..aaec26e9f4e 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormAIController.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormAIController.java @@ -27,6 +27,7 @@ import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; +import java.util.function.BiFunction; import java.util.function.Function; import org.slf4j.Logger; @@ -37,6 +38,7 @@ import com.vaadin.flow.component.HasComponents; import com.vaadin.flow.component.HasEnabled; import com.vaadin.flow.component.HasValue; +import com.vaadin.flow.component.ItemLabelGenerator; import com.vaadin.flow.component.ai.form.FormAITools.FormFieldDescriptor; import com.vaadin.flow.component.ai.form.FormValueConverter.RejectedValueException; import com.vaadin.flow.component.ai.orchestrator.AIController; @@ -87,7 +89,10 @@ * {@link #fieldValueOptions(ValueOptions, Function) two-argument overload} also * accepts a label-to-value converter, which the controller applies per label; * for multi-select fields the resolved elements are then aggregated into a - * {@link LinkedHashSet} before {@link HasValue#setValue}. + * {@link LinkedHashSet} before {@link HasValue#setValue}. LLM-facing labels for + * the registered items are derived from the field's + * {@code setItemLabelGenerator(...)} by default; see {@link ValueOptions} for + * the full resolution chain. *

* *

@@ -339,10 +344,10 @@ public FormAIController describeField(HasValue field, } /** - * Registers a known set of labels for a {@link String}-typed field. The - * labels are presented to the LLM as the field's choices. No converter is - * needed — the chosen label is itself the value written to the field. For - * any non-{@link String} value type, use + * Registers a known set of options for a {@link String}-typed field. Each + * item is also the label the LLM sees — the chosen label is itself the + * value written to the field, with no converter needed. For any + * non-{@link String} value type, use * {@link #fieldValueOptions(ValueOptions, Function) the two-argument * overload} to supply a converter; this is enforced at compile time. *

@@ -353,14 +358,14 @@ public FormAIController describeField(HasValue field, * * @param config * the field's options registration, not {@code null}; must have - * its label source set via either + * its item source set via either * {@link ValueOptions#options(Collection)} or * {@link ValueOptions#options(BiFunction)} * @return this controller, for chaining * @throws NullPointerException * if {@code config} is {@code null} * @throws IllegalArgumentException - * if the registration has no label source set; if the developer + * if the registration has no item source set; if the developer * routed a {@code MultiSelect} field through the single-value * {@code forField} overload (upcast reference); or if the * field's value type is a Collection but the field does not @@ -372,13 +377,17 @@ public FormAIController fieldValueOptions(ValueOptions config) { } /** - * Registers a known set of labels for a field, paired with a converter that - * resolves a chosen label to the field's value type. When the LLM picks a - * label, the controller calls {@code toValue} on it and writes the result - * to the field. If {@code toValue} returns {@code null} or throws — for - * example because the LLM picked a label the converter does not recognize — - * the write is rejected back to the LLM with a reason and the model can - * correct on the next turn. + * Registers a known set of items for a field, paired with a converter that + * resolves a chosen label to the field's value type. The controller derives + * the LLM-facing label for each item through this chain: an explicit + * {@link ValueOptions#itemLabelGenerator(ItemLabelGenerator)} if set, + * otherwise the field's own {@code setItemLabelGenerator(...)} (read + * reflectively), otherwise {@link String#valueOf(Object)} as a last resort. + * When the LLM picks a label, the controller calls {@code toValue} on it + * and writes the result to the field. If {@code toValue} returns + * {@code null} or throws — for example because the LLM picked a label the + * converter does not recognize — the write is rejected back to the LLM with + * a reason and the model can correct on the next turn. *

* A typical converter delegates to a service or repository that looks the * domain object up by its display name, for example @@ -394,20 +403,20 @@ public FormAIController fieldValueOptions(ValueOptions config) { * * @param config * the field's options registration, not {@code null}; must have - * its label source set via either + * its item source set via either * {@link ValueOptions#options(Collection)} or * {@link ValueOptions#options(BiFunction)} * @param toValue * converts a chosen label to one element of the field's value * type, not {@code null} * @param - * the per-label item type — the field's value type for - * single-value fields, the per-element type for multi-select + * the item type — the field's value type for single-value + * fields, the per-element type for multi-select * @return this controller, for chaining * @throws NullPointerException * if {@code config} or {@code toValue} is {@code null} * @throws IllegalArgumentException - * if the registration has no label source set; if the developer + * if the registration has no item source set; if the developer * routed a {@code MultiSelect} field through the single-value * {@code forField} overload (upcast reference); or if the * field's value type is a Collection but the field does not @@ -453,19 +462,66 @@ private FormAIController applyValueOptions(ValueOptions config, + "fields must implement MultiSelect to be " + "registered via fieldValueOptions(...)."); } + var labeler = resolveItemLabeler(config.field(), + config.itemLabelGenerator()); var hints = hintsFor(config.field()); hints.valueOptionsToValue = toValue; + hints.itemLabelGenerator = labeler; if (fixed != null) { - hints.valueOptionsQuery = (filter, limit) -> filterAndLimit(fixed, - filter, limit); + @SuppressWarnings({ "unchecked", "rawtypes" }) + List items = (List) fixed; + hints.valueOptionsQuery = (filter, limit) -> filterAndLimit(items, + filter, limit, labeler); hints.fixedOptions = true; } else { - hints.valueOptionsQuery = query; + @SuppressWarnings({ "unchecked", "rawtypes" }) + BiFunction> rawQuery = (BiFunction) query; + hints.valueOptionsQuery = (filter, limit) -> rawQuery + .apply(filter, limit).stream().map(labeler).toList(); hints.fixedOptions = false; } return this; } + /** + * Resolves the V-to-label function for one valueOptions registration. + * Priority: an explicit {@link ItemLabelGenerator} on the registration, + * otherwise the field's own {@code getItemLabelGenerator()} (via + * {@link FormValueConverter#renderItem}, which also covers the + * {@link String#valueOf} fallback). + */ + private static Function resolveItemLabeler( + HasValue field, ItemLabelGenerator explicit) { + if (explicit != null) { + @SuppressWarnings({ "unchecked", "rawtypes" }) + ItemLabelGenerator typed = (ItemLabelGenerator) explicit; + return item -> applyItemLabeler(typed, item); + } + return item -> FormValueConverter.renderItem(field, item); + } + + /** + * Applies an explicit per-item label generator, falling back to + * {@link String#valueOf(Object)} for {@code null} items, {@code null} + * labels, and labelers that throw. Mirrors the safety guarantees of + * {@link FormValueConverter#renderItem} so a misbehaving label generator + * cannot collapse the whole tool call. + */ + private static String applyItemLabeler(ItemLabelGenerator labeler, + Object item) { + if (item == null) { + return ""; + } + try { + var label = labeler.apply(item); + return label != null ? label : String.valueOf(item); + } catch (Exception ex) { + LOGGER.warn("Item label generator threw for {}", item.getClass(), + ex); + return String.valueOf(item); + } + } + /** * Hides the given field from the LLM. The field's value is never exposed to * the LLM, the LLM cannot write to it, and it is not locked during a fill. @@ -941,9 +997,9 @@ private static String getOrCreateId(HasValue field) { return id; } - private static List filterAndLimit(List source, - String filter, int limit) { - var stream = source.stream(); + private static List filterAndLimit(List source, + String filter, int limit, Function labeler) { + var stream = source.stream().map(labeler); if (filter != null && !filter.isEmpty()) { var needle = filter.toLowerCase(Locale.ROOT); stream = stream diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldHints.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldHints.java index 224e900b107..3840784f9be 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldHints.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldHints.java @@ -24,13 +24,17 @@ * field's opaque id. *

* Set by {@link FormAIController#fieldValueOptions(ValueOptions) - * controller.fieldValueOptions(...)}: {@link #valueOptionsQuery} carries the - * filter callback (or a fixed-options snapshot wrapped in one), - * {@link #fixedOptions} flags whether the schema should render the options as - * {@code enum} or {@code queryable}, and {@link #valueOptionsToValue} resolves - * one label to one element. For multi-select fields the controller wraps the - * resolved elements into a {@link java.util.LinkedHashSet} before - * {@code setValue}; the hint state is the same shape in both cases. + * controller.fieldValueOptions(...)}: {@link #valueOptionsQuery} returns the + * labels the LLM sees for a given filter (the controller wraps the + * {@link ValueOptions}' item source + label generator into this single + * label-producing callback at registration time), {@link #fixedOptions} flags + * whether the schema should render the options as {@code enum} or + * {@code queryable}, {@link #valueOptionsToValue} resolves one label back to + * one element, and {@link #itemLabelGenerator} renders the field's current + * value through the same labeler so the value string matches the labels the LLM + * was offered. For multi-select fields the controller wraps the resolved + * elements into a {@link java.util.LinkedHashSet} before {@code setValue}; the + * hint state is the same shape in both cases. * * @author Vaadin Ltd */ @@ -39,6 +43,18 @@ final class FormFieldHints { String description; BiFunction> valueOptionsQuery; Function valueOptionsToValue; + /** + * Item-to-label function used to render the current value when + * value-options is registered. Resolved at registration to the explicit + * {@link ValueOptions#itemLabelGenerator(com.vaadin.flow.component.ItemLabelGenerator)} + * or to a delegate that defers to {@link FormValueConverter#renderItem} + * (field's own {@code getItemLabelGenerator()}, then + * {@link String#valueOf(Object)}). Non-{@code null} whenever + * {@link #valueOptionsQuery} is set, so the schema's value string agrees + * with the labels emitted in the {@code enum} / {@code query_field_options} + * payloads. + */ + Function itemLabelGenerator; /** * {@code true} when the field was registered with the fixed-options * variant; {@code false} when registered with a query callback or with no diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldSchema.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldSchema.java index cc328852713..47001c77bcd 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldSchema.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldSchema.java @@ -157,8 +157,8 @@ private static void applySelectionOptions(ObjectNode target, return; } var arr = target.putArray("enum"); - items.stream().limit(ENUM_MAX_ITEMS).forEach( - item -> arr.add(FormValueConverter.renderItem(field, item))); + items.stream().limit(ENUM_MAX_ITEMS) + .forEach(item -> arr.add(renderItem(field, hints, item))); } private static void applyValue(ObjectNode node, HasValue field, @@ -173,7 +173,7 @@ private static void applyValue(ObjectNode node, HasValue field, // string so the two halves of the payload agree. if (hasValueOptions(hints) && type != FormFieldType.SINGLE_SELECT && type != FormFieldType.MULTI_SELECT) { - node.put(FIELD_VALUE, FormValueConverter.renderItem(field, value)); + node.put(FIELD_VALUE, renderItem(field, hints, value)); return; } switch (type) { @@ -188,12 +188,29 @@ private static void applyValue(ObjectNode node, HasValue field, node.put(FIELD_VALUE, ((BigDecimal) value).toPlainString()); case BOOLEAN -> node.put(FIELD_VALUE, (Boolean) value); case SINGLE_SELECT -> - node.put(FIELD_VALUE, FormValueConverter.renderItem(field, value)); - case MULTI_SELECT -> applyMultiSelectValue(node, field, value); + node.put(FIELD_VALUE, renderItem(field, hints, value)); + case MULTI_SELECT -> applyMultiSelectValue(node, field, hints, value); default -> node.put(FIELD_VALUE, value.toString()); } } + /** + * Renders one item to the LLM-facing label. Prefers the resolved + * valueOptions item-label generator (which honours an explicit + * {@link ValueOptions#itemLabelGenerator(com.vaadin.flow.component.ItemLabelGenerator)} + * over the field's own generator) so the value string agrees with the + * labels surfaced in {@code enum} / {@code query_field_options}. Falls back + * to {@link FormValueConverter#renderItem} when no valueOptions hint is + * registered. + */ + private static String renderItem(HasValue field, FormFieldHints hints, + Object item) { + if (hints != null && hints.itemLabelGenerator != null) { + return hints.itemLabelGenerator.apply(item); + } + return FormValueConverter.renderItem(field, item); + } + private static void applyNumberValue(ObjectNode node, Object value) { // NaN / ±Infinity are valid Java doubles but not legal JSON numbers; // surface as null rather than corrupting the payload with a @@ -217,7 +234,7 @@ private static void applyIntegerValue(ObjectNode node, Object value) { } private static void applyMultiSelectValue(ObjectNode node, - HasValue field, Object value) { + HasValue field, FormFieldHints hints, Object value) { // MULTI_SELECT is only assigned when the field implements MultiSelect, // whose contract guarantees getValue() returns a Set. A non-Collection // value would be a contract violation and produces an empty array @@ -225,7 +242,7 @@ private static void applyMultiSelectValue(ObjectNode node, var arr = node.putArray(FIELD_VALUE); if (value instanceof Collection coll) { for (var v : coll) { - arr.add(FormValueConverter.renderItem(field, v)); + arr.add(renderItem(field, hints, v)); } } } diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/ValueOptions.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/ValueOptions.java index ebae7d1f3de..68a452c8ea8 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/ValueOptions.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/ValueOptions.java @@ -24,36 +24,46 @@ import com.vaadin.flow.component.Component; import com.vaadin.flow.component.HasValue; +import com.vaadin.flow.component.ItemLabelGenerator; import com.vaadin.flow.data.selection.MultiSelect; /** - * Per-field registration of the labels the LLM may pick from for a given field. - * The label set is either fixed ({@link #options(Collection)}) or supplied on + * Per-field registration of the items the LLM may pick from for a given field. + * The item set is either fixed ({@link #options(Collection)}) or supplied on * demand by a callback ({@link #options(BiFunction)}); exactly one of the two * must be set, with the last call winning. Pass the configured registration to * {@link FormAIController#fieldValueOptions(ValueOptions) - * controller.fieldValueOptions(...)} to apply it; the labels are then presented - * to the LLM as the field's choices. + * controller.fieldValueOptions(...)} to apply it; the items are then rendered + * to labels and presented to the LLM as the field's choices. *

- * Labels are always {@link String} values, regardless of the field's value - * type. For a non-{@link String} field, supply a label-to-value converter - * through {@link FormAIController#fieldValueOptions(ValueOptions, Function) + * Items carry the field's value type, so a {@code ComboBox} + * registration passes {@code Project} items, not pre-rendered label strings. + * The controller derives the LLM-facing label for each item through this chain: + * an explicit {@link #itemLabelGenerator(ItemLabelGenerator)} if set, otherwise + * the field's own {@code setItemLabelGenerator(...)} (read reflectively), + * otherwise {@link String#valueOf(Object)} as a last resort. The label produced + * by this chain is what the LLM sees, picks from, and sends back to the + * {@code toValue} converter — so the field's UI label generator drives the AI + * labels automatically when set. + *

+ * For a non-{@link String} field, supply a label-to-value converter through + * {@link FormAIController#fieldValueOptions(ValueOptions, Function) * fieldValueOptions(config, toValue)} — the converter resolves a chosen label - * to the field's value type before the controller writes it. + * back to the field's value type before the controller writes it. * - * @param - * the per-label item type the converter must produce — the field's - * value type for single-value fields, the per-element type for - * multi-select fields + * @param + * the item type — the field's value type for single-value fields, + * the per-element type for multi-select fields * * @author Vaadin Ltd */ -public final class ValueOptions { +public final class ValueOptions { private final HasValue field; private final boolean multi; - private BiFunction> query; - private List fixedOptions; + private BiFunction> query; + private List fixedOptions; + private ItemLabelGenerator itemLabelGenerator; private ValueOptions(HasValue field, boolean multi) { this.field = field; @@ -108,22 +118,23 @@ public static ValueOptions forField( } /** - * Sets a fixed label list. A defensive copy is taken so later mutations of + * Sets a fixed item list. A defensive copy is taken so later mutations of * the caller's collection have no effect. Use this when the option set is * known up front and small enough to enumerate; for large or dynamic sets * use {@link #options(BiFunction)} instead. Mutually exclusive with * {@link #options(BiFunction)} — calling either clears the other. * * @param options - * the labels the LLM may pick from, not {@code null} and not - * empty + * the items the LLM may pick from, not {@code null} and not + * empty; each item is rendered to an LLM-facing label through + * the chain documented on the class JavaDoc * @return this registration, for chaining * @throws IllegalArgumentException * if {@code options} is empty — registering an empty fixed list * leaves the field un-fillable and is always a developer * mistake */ - public ValueOptions options(Collection options) { + public ValueOptions options(Collection options) { Objects.requireNonNull(options, "Options must not be null"); if (options.isEmpty()) { throw new IllegalArgumentException("Options must not be empty"); @@ -138,25 +149,49 @@ public ValueOptions options(Collection options) { * field's options. Use this when the option set is too large or too dynamic * for a fixed list via {@link #options(Collection)} — for example options * that come from a database query or a remote service. The callback returns - * the labels the LLM may pick from; labels are always {@link String} - * values, regardless of the field's value type. Mutually exclusive with - * {@link #options(Collection)} — calling either clears the other. + * items the LLM may pick from; each item is rendered to an LLM-facing label + * through the chain documented on the class JavaDoc. Mutually exclusive + * with {@link #options(Collection)} — calling either clears the other. * * @param query * invoked with two arguments: a filter string the LLM picked, - * and a positive limit on how many labels to return. Returns the - * matching labels in display order, not {@code null} (an empty + * and a positive limit on how many items to return. Returns the + * matching items in display order, not {@code null} (an empty * list signals "no matches" to the LLM) * @return this registration, for chaining */ - public ValueOptions options( - BiFunction> query) { + public ValueOptions options(BiFunction> query) { Objects.requireNonNull(query, "Options query must not be null"); this.query = query; this.fixedOptions = null; return this; } + /** + * Sets the item-to-label function the controller uses to derive the + * LLM-facing label for each item. Optional — when unset, the controller + * falls back to the field's own {@code getItemLabelGenerator()} (read + * reflectively), then to {@link String#valueOf(Object)} as a last resort. + * Set this when the field's UI label generator is absent or when the LLM + * needs a different label from the UI (for example a code rather than a + * display name). Calling this multiple times overwrites the previous value. + *

+ * Two items rendering to the same label are not an error: the duplicate + * appears verbatim in the LLM's option list, and when the LLM picks the + * shared label the supplied {@code toValue} converter decides which item is + * written. Emit unique labels per item if disambiguation matters. + * + * @param generator + * the per-item label generator, not {@code null} + * @return this registration, for chaining + */ + public ValueOptions itemLabelGenerator(ItemLabelGenerator generator) { + Objects.requireNonNull(generator, + "Item label generator must not be null"); + this.itemLabelGenerator = generator; + return this; + } + HasValue field() { return field; } @@ -165,11 +200,15 @@ boolean isMulti() { return multi; } - BiFunction> query() { + BiFunction> query() { return query; } - List fixedOptions() { + List fixedOptions() { return fixedOptions; } + + ItemLabelGenerator itemLabelGenerator() { + return itemLabelGenerator; + } } diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FillFormToolTest.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FillFormToolTest.java index daa59486e4d..dfdb8f2709f 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FillFormToolTest.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FillFormToolTest.java @@ -1292,16 +1292,18 @@ void fillForm_singleSelect_writesResolvedValueViaValueOptionsToValue() { // The LLM speaks in labels for SINGLE_SELECT fields with // fieldValueOptions registered. The tool must apply toValue so the // field gets the domain instance, not the raw label string. + var apollo = new Project("P-1", "Apollo"); + var projects = Map.of("Apollo", apollo); var field = new SingleSelectField(); - var projects = Map.of("Apollo", new Project("P-1", "Apollo")); + field.setItemLabelGenerator(Project::name); var controller = newController(field); controller.fieldValueOptions(ValueOptions.forField(field) - .options((filter, limit) -> List.of("Apollo")), projects::get); + .options((filter, limit) -> List.of(apollo)), projects::get); controller.onRequest(); var result = fillFormResult(controller, payload(field, "\"Apollo\"")); - Assertions.assertEquals(new Project("P-1", "Apollo"), field.getValue(), + Assertions.assertEquals(apollo, field.getValue(), "Field must receive the resolved domain object, not the " + "raw label string"); Assertions.assertTrue(success(result)); @@ -1397,9 +1399,13 @@ void fillForm_singleSelect_unknownLabelIsRejected() { // match any option". The orchestrator must reject rather than // pass null to setValue (which would silently clear the field). var field = new SingleSelectField(); + field.setItemLabelGenerator(Project::name); var controller = newController(field); - controller.fieldValueOptions(ValueOptions.forField(field) - .options((filter, limit) -> List.of("Apollo")), label -> null); + controller.fieldValueOptions( + ValueOptions.forField(field) + .options((filter, limit) -> List + .of(new Project("P-1", "Apollo"))), + label -> null); controller.onRequest(); var result = fillFormResult(controller, payload(field, "\"Unknown\"")); @@ -1418,19 +1424,21 @@ void fillForm_multiSelect_writesResolvedSetViaValueOptions() { // fieldValueOptions on a MultiSelectField takes a single-item Function; // the converter accumulates per-label items into the Set // value type via an internal LinkedHashSet. + var apollo = new Project("Apollo", "Apollo"); + var vega = new Project("Vega", "Vega"); var field = new MultiSelectField(); + field.setItemLabelGenerator(Project::name); var controller = newController(field); controller.fieldValueOptions( ValueOptions.forField(field) - .options((filter, limit) -> List.of("Apollo", "Vega")), + .options((filter, limit) -> List.of(apollo, vega)), label -> new Project(label, label)); controller.onRequest(); var result = fillFormResult(controller, payload(field, "[\"Apollo\", \"Vega\"]")); - Assertions.assertEquals(Set.of(new Project("Apollo", "Apollo"), - new Project("Vega", "Vega")), field.getValue()); + Assertions.assertEquals(Set.of(apollo, vega), field.getValue()); Assertions.assertTrue(success(result)); } @@ -1518,6 +1526,7 @@ void fillForm_multiSelect_emptyArrayClearsField() { // builds an empty LinkedHashSet so the field's setValue receives // an empty Set. var field = new MultiSelectField(); + field.setItemLabelGenerator(Project::name); var existing = new Project("X", "X"); field.setValue(Set.of(existing)); var controller = newController(field); @@ -1536,13 +1545,15 @@ void fillForm_multiSelect_nonArrayPayloadIsRejected() { // The LLM sends a bare string for a multi-select field. The // converter enforces the array shape so a stray scalar payload // doesn't reach setValue. + var apollo = new Project("Apollo", "Apollo"); var field = new MultiSelectField(); + field.setItemLabelGenerator(Project::name); var existing = new Project("X", "X"); field.setValue(Set.of(existing)); var controller = newController(field); controller.fieldValueOptions( ValueOptions.forField(field) - .options((filter, limit) -> List.of("Apollo")), + .options((filter, limit) -> List.of(apollo)), label -> new Project(label, label)); controller.onRequest(); @@ -1562,22 +1573,24 @@ void fillForm_multiSelect_reregistrationOverwritesPriorOptions() { // fieldValueOptions called twice on the same MultiSelect field — the // second call wins, including switching from fixed to queryable // and replacing the toValue function. + var first = new Project("v1", "First"); + var second = new Project("v2", "Second"); var field = new MultiSelectField(); + field.setItemLabelGenerator(Project::name); var controller = newController(field); controller.fieldValueOptions( - ValueOptions.forField(field).options(List.of("First")), + ValueOptions.forField(field).options(List.of(first)), label -> new Project("v1", label)); controller.fieldValueOptions( ValueOptions.forField(field) - .options((filter, limit) -> List.of("Second")), + .options((filter, limit) -> List.of(second)), label -> new Project("v2", label)); controller.onRequest(); var result = fillFormResult(controller, payload(field, "[\"Second\"]")); Assertions.assertTrue(success(result)); - Assertions.assertEquals(Set.of(new Project("v2", "Second")), - field.getValue(), + Assertions.assertEquals(Set.of(second), field.getValue(), "Re-registration must hand the LLM-supplied label to the " + "second toValue, not the first"); } @@ -1592,8 +1605,8 @@ void fillForm_fieldValueOptionsOnPrimitiveTypeUsesToValueNotTypeDrivenParsing() var field = new IntField(); var controller = newController(field); controller.fieldValueOptions( - ValueOptions.forField(field) - .options((filter, limit) -> List.of("low", "high")), + ValueOptions.forField(field).options(List.of(1, 10)) + .itemLabelGenerator(v -> v == 1 ? "low" : "high"), label -> "low".equals(label) ? 1 : 10); controller.onRequest(); diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormAIControllerTest.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormAIControllerTest.java index 9a1d6c9fe79..70bda2f16f3 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormAIControllerTest.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormAIControllerTest.java @@ -514,11 +514,13 @@ void hintMethodsRejectNullPayload() { (java.util.function.BiFunction>) null)); Assertions.assertThrows(NullPointerException.class, () -> config.options((java.util.Collection) null)); + Assertions.assertThrows(NullPointerException.class, + () -> config.itemLabelGenerator(null)); // A null converter passed to the controller's two-argument // overload throws NPE. Assertions.assertThrows(NullPointerException.class, () -> controller.fieldValueOptions(ValueOptions - .forField(new IntField()).options(List.of("a")), + .forField(new IntField()).options(List.of(1)), null)); // An empty fixed-options list leaves the field un-fillable; // rejected at the options() call site. @@ -536,14 +538,15 @@ void hintMethodsRejectNullPayload() { @Test void fieldValueOptionsAcceptsNonStringFieldWithToValueConverter() { - // fieldValueOptions is generic over the field's value type — verify - // that an Integer-valued field can be registered with a label - // -> Integer converter, and that the labels still flow through - // the query tool unchanged. + // fieldValueOptions is generic over the field's value type — an + // Integer-valued field carries Integer items, and the + // String.valueOf fallback renders the LLM-facing labels when + // the field has no item-label generator of its own. var field = new IntField(); var controller = new FormAIController(new Div(field)); - controller.fieldValueOptions(ValueOptions.forField(field) - .options(List.of("1", "2", "3")), Integer::parseInt); + controller.fieldValueOptions( + ValueOptions.forField(field).options(List.of(1, 2, 3)), + Integer::parseInt); controller.onRequest(); Assertions.assertEquals("1\n2\n3\n", @@ -574,8 +577,7 @@ void identityToValueLetsStringFieldsUseLabelsDirectly() { @Test void reregisteringWithBiFunctionClearsPriorFixedOptionsFlag() { // Each fieldValueOptions call replaces the previous registration - // for - // the same field. The fixed-options variant sets a flag that + // for the same field. The fixed-options variant sets a flag that // makes the schema render options as 'enum'; re-registering with // a query callback must reset that flag so the schema rendering // matches the new registration (queryable, not enum). @@ -598,6 +600,41 @@ void reregisteringWithBiFunctionClearsPriorFixedOptionsFlag() { + "a BiFunction, got: " + field); } + @Test + void reregisteringValueOptionsOverwritesItemLabelGenerator() { + // Each fieldValueOptions call replaces the previous registration + // in full — including the item-label generator — so a stale + // labeler cannot survive a re-registration. Asserted on both the + // enum block (the wrapped query path) and the schema's value + // string (the value-rendering path) so a half-overwrite of one + // but not the other is caught. + var alpha = new FormTestFields.Project("P-1", "Alpha"); + var combo = new FormTestFields.SingleSelectField(); + combo.setValue(alpha); + var controller = new FormAIController(new Div(combo)); + controller.fieldValueOptions( + ValueOptions.forField(combo).options(List.of(alpha)) + .itemLabelGenerator(FormTestFields.Project::code), + label -> alpha); + controller.fieldValueOptions( + ValueOptions.forField(combo).options(List.of(alpha)) + .itemLabelGenerator(FormTestFields.Project::name), + label -> alpha); + + var f = json(findTool(controller.getTools(), "get_form_state") + .execute(JacksonUtils.createObjectNode())).path("fields") + .get(0); + var labels = new java.util.ArrayList(); + f.path("enum").forEach(n -> labels.add(n.asString())); + + Assertions.assertEquals(List.of("Alpha"), labels, + "Second registration's labeler must drive the enum " + + "block; got: " + labels); + Assertions.assertEquals("Alpha", f.path("value").asString(), + "Second registration's labeler must drive the value " + + "rendering; got: " + f.path("value")); + } + @Test void fieldValueOptionsForFieldOnUpcastMultiSelectReferenceThrowsIllegalArgument() { // A MultiSelect statically typed as such picks the MultiSelect- @@ -612,7 +649,8 @@ void fieldValueOptionsForFieldOnUpcastMultiSelectReferenceThrowsIllegalArgument( var controller = new FormAIController(new Div(multiSelect)); var ex = Assertions.assertThrows(IllegalArgumentException.class, () -> controller.fieldValueOptions( - ValueOptions.forField(upcast).options(List.of("a")), + ValueOptions.forField(upcast) + .options(List.of(java.util.Set.of("a"))), label -> java.util.Set.of(label))); Assertions.assertTrue(ex.getMessage().contains("MultiSelect"), "Rejection must name MultiSelect so the developer can " @@ -627,10 +665,14 @@ void fieldValueOptionsRejectsCollectionValuedFieldNotImplementingMultiSelect() { // and the controller refuses to register them. var field = new FormTestFields.CollectionWithoutMultiSelectField(); var controller = new FormAIController(new Div(field)); - var ex = Assertions.assertThrows(IllegalArgumentException.class, - () -> controller.fieldValueOptions( - ValueOptions.forField(field).options(List.of("a")), - label -> List.of(label))); + var ex = Assertions + .assertThrows(IllegalArgumentException.class, + () -> controller + .fieldValueOptions( + ValueOptions.forField(field) + .options(List + .of(List.of("a"))), + label -> List.of(label))); Assertions.assertTrue( ex.getMessage().contains("Collection") && ex.getMessage().contains("MultiSelect"), @@ -648,7 +690,7 @@ void fieldValueOptionsAcceptsTypedMultiSelectFieldWithNonStringConverter() { var field = new FormTestFields.MultiSelectField(); var controller = new FormAIController(new Div(field)); Assertions.assertDoesNotThrow(() -> controller.fieldValueOptions( - ValueOptions.forField(field).options(List.of("1", "2")), + ValueOptions.forField(field).options(List.of(1, 2)), Integer::parseInt)); } diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormStateToolTest.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormStateToolTest.java index db1e19b694d..646dd7bfe19 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormStateToolTest.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormStateToolTest.java @@ -677,14 +677,16 @@ void getFormStateSingleSelectWithValueOptionsRendersValueAsLabel() { // selection-aware path must keep using the field's own label // renderer for the current value, not the generic fieldValueOptions // value-rewrite branch. + var alpha = new Project("P-1", "Alpha"); + var beta = new Project("P-2", "Beta"); var combo = new SingleSelectField(); - combo.setItems(new Project("P-1", "Alpha"), new Project("P-2", "Beta")); + combo.setItems(alpha, beta); combo.setItemLabelGenerator(p -> p.code() + " " + p.name()); - combo.setValue(new Project("P-2", "Beta")); + combo.setValue(beta); var controller = new FormAIController(new Div(combo)); controller.fieldValueOptions( - ValueOptions.forField(combo).options( - (filter, limit) -> List.of("P-1 Alpha", "P-2 Beta")), + ValueOptions.forField(combo) + .options((filter, limit) -> List.of(alpha, beta)), label -> null); var f = formStateFields(controller).get(0); @@ -877,6 +879,172 @@ void getFormStateUsesFallbackWhenLabelGeneratorReturnsNull() { Assertions.assertFalse(f.path("value").isNull()); } + @Test + void getFormStateValueOptionsExplicitLabelerOverridesFieldsLabelGenerator() { + // The labeler chain prefers an explicit itemLabelGenerator on + // ValueOptions over the field's setItemLabelGenerator, so the LLM + // can see different labels than the UI when an application needs + // to (e.g. a code instead of a display name). + var alpha = new Project("P-1", "Alpha"); + var beta = new Project("P-2", "Beta"); + var combo = new SingleSelectField(); + combo.setItemLabelGenerator(Project::name); + var controller = new FormAIController(new Div(combo)); + controller.fieldValueOptions( + ValueOptions.forField(combo).options(List.of(alpha, beta)) + .itemLabelGenerator(Project::code), + label -> alpha); + + var f = formStateFields(controller).get(0); + var labels = new ArrayList(); + f.path("enum").forEach(n -> labels.add(n.asString())); + + Assertions.assertEquals(List.of("P-1", "P-2"), labels, + "Explicit ValueOptions labeler must override the field's " + + "setItemLabelGenerator for the enum block, got: " + + labels); + } + + @Test + void getFormStateValueOptionsUsesFieldsLabelGeneratorWhenNoExplicit() { + // Second leg of the labeler chain: with no explicit labeler on + // ValueOptions, the controller reuses the field's own + // setItemLabelGenerator so the LLM-facing labels match what the + // UI already shows. No duplication of the V-to-label mapping. + var alpha = new Project("P-1", "Alpha"); + var beta = new Project("P-2", "Beta"); + var combo = new SingleSelectField(); + combo.setItemLabelGenerator(Project::name); + var controller = new FormAIController(new Div(combo)); + controller.fieldValueOptions( + ValueOptions.forField(combo).options(List.of(alpha, beta)), + label -> alpha); + + var f = formStateFields(controller).get(0); + var labels = new ArrayList(); + f.path("enum").forEach(n -> labels.add(n.asString())); + + Assertions.assertEquals(List.of("Alpha", "Beta"), labels); + } + + @Test + void getFormStateValueOptionsFallsBackToToStringWithoutAnyLabelGenerator() { + // Last leg of the labeler chain: no explicit labeler and no + // field-level setItemLabelGenerator falls back to + // String.valueOf(item) so Integer / String / enum-typed items + // self-label with no developer setup. + var field = new IntField(); + var controller = new FormAIController(new Div(field)); + controller.fieldValueOptions( + ValueOptions.forField(field).options(List.of(1, 2, 3)), + Integer::parseInt); + + var f = formStateFields(controller).get(0); + var labels = new ArrayList(); + f.path("enum").forEach(n -> labels.add(n.asString())); + + Assertions.assertEquals(List.of("1", "2", "3"), labels); + } + + @Test + void getFormStateValueOptionsExplicitLabelerAlsoDrivesValueRendering() { + // When an explicit ValueOptions labeler is set, the current value + // emitted in the schema must use the same labeler so the LLM sees + // one label set across both reads (enum) and the current value — + // otherwise the LLM would pick a label from the enum that doesn't + // match what is reported as the value. + var alpha = new Project("P-1", "Alpha"); + var combo = new SingleSelectField(); + combo.setItemLabelGenerator(Project::name); + combo.setValue(alpha); + var controller = new FormAIController(new Div(combo)); + controller + .fieldValueOptions( + ValueOptions.forField(combo).options(List.of(alpha)) + .itemLabelGenerator(Project::code), + label -> alpha); + + var f = formStateFields(controller).get(0); + + Assertions.assertEquals("P-1", f.path("value").asString(), + "Current value must render through the explicit ValueOptions " + + "labeler, not the field's UI labeler, got: " + + f.path("value")); + } + + @Test + void getFormStateValueOptionsExplicitLabelerThrowingFallsBackToToString() { + // A throwing item-label generator must not collapse the + // get_form_state call; the controller catches the throw and falls + // back to String.valueOf for the offending item so the LLM still + // sees the entry. + var alpha = new Project("P-1", "Alpha"); + var combo = new SingleSelectField(); + var controller = new FormAIController(new Div(combo)); + controller.fieldValueOptions(ValueOptions.forField(combo) + .options(List.of(alpha)).itemLabelGenerator(p -> { + throw new RuntimeException("boom"); + }), label -> alpha); + + var f = formStateFields(controller).get(0); + var labels = new ArrayList(); + f.path("enum").forEach(n -> labels.add(n.asString())); + + Assertions.assertEquals(List.of(String.valueOf(alpha)), labels, + "Throwing labeler must fall back to String.valueOf(item) " + + "for the offending item; got: " + labels); + } + + @Test + void getFormStateValueOptionsExplicitLabelerReturningNullFallsBackToToString() { + // A null-returning item-label generator falls back to + // String.valueOf(item) so the LLM never receives a null or empty + // label in the enum block. + var alpha = new Project("P-1", "Alpha"); + var combo = new SingleSelectField(); + var controller = new FormAIController(new Div(combo)); + controller.fieldValueOptions(ValueOptions.forField(combo) + .options(List.of(alpha)).itemLabelGenerator(p -> null), + label -> alpha); + + var f = formStateFields(controller).get(0); + var labels = new ArrayList(); + f.path("enum").forEach(n -> labels.add(n.asString())); + + Assertions.assertEquals(List.of(String.valueOf(alpha)), labels, + "Null label from explicit generator must fall back to " + + "String.valueOf(item); got: " + labels); + } + + @Test + void getFormStateMultiSelectValueOptionsLabelerAppliesToItemsEnumAndValue() { + // Multi-select symmetry: the explicit labeler drives both + // items.enum and the per-element value strings, so reads and + // writes agree across both halves of the payload. + var apollo = new Project("APL", "Apollo"); + var vega = new Project("VGA", "Vega"); + var multi = new MultiSelectField(); + multi.setItemLabelGenerator(Project::name); + multi.setValue(Set.of(apollo)); + var controller = new FormAIController(new Div(multi)); + controller.fieldValueOptions( + ValueOptions.forField(multi).options(List.of(apollo, vega)) + .itemLabelGenerator(Project::code), + label -> apollo); + + var f = formStateFields(controller).get(0); + var enumLabels = new ArrayList(); + f.path("items").path("enum").forEach(n -> enumLabels.add(n.asString())); + var valueLabels = new ArrayList(); + f.path("value").forEach(n -> valueLabels.add(n.asString())); + + Assertions.assertEquals(List.of("APL", "VGA"), enumLabels, + "items.enum must use the explicit labeler, got: " + enumLabels); + Assertions.assertEquals(List.of("APL"), valueLabels, + "Selected value must use the explicit labeler, got: " + + valueLabels); + } + @Test void getFormStateExposesListDataProviderItemsAsEnum() { var combo = new SingleSelectField(); @@ -1026,7 +1194,7 @@ void getFormStateRendersStringValueWhenValueOptionsOverrideType() { field.setValue(2); var controller = new FormAIController(new Div(field)); controller.fieldValueOptions( - ValueOptions.forField(field).options(List.of("1", "2", "3")), + ValueOptions.forField(field).options(List.of(1, 2, 3)), Integer::parseInt); var f = formStateFields(controller).get(0); diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/QueryFieldOptionsToolTest.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/QueryFieldOptionsToolTest.java index ae003435352..c7c026c481b 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/QueryFieldOptionsToolTest.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/QueryFieldOptionsToolTest.java @@ -29,6 +29,8 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import com.vaadin.flow.component.ai.form.FormTestFields.Project; +import com.vaadin.flow.component.ai.form.FormTestFields.SingleSelectField; import com.vaadin.flow.component.ai.form.FormTestFields.TestField; import com.vaadin.flow.component.html.Div; @@ -42,10 +44,9 @@ class QueryFieldOptionsToolTest { @Test void queryFieldOptionsReturnsRegisteredOptions() { // End-to-end: register fieldValueOptions on a field, then drive the - // tool - // the way an LLM would — call getTools().execute(...) with the field - // id. Pins the wiring from fieldValueOptions registration through - // ToolCallbacks to the query function. + // tool the way an LLM would — call getTools().execute(...) with the + // field id. Pins the wiring from fieldValueOptions registration + // through ToolCallbacks to the query function. var field = new TestField(); var controller = new FormAIController(new Div(field)); controller.fieldValueOptions(ValueOptions.forField(field) @@ -67,9 +68,8 @@ void queryFieldOptionsReportsUnknownFieldId() { // When the LLM sends a field id the controller doesn't recognize // (hallucinated, stale, or for a field that was registered with // describeField()/ignoreField() but not fieldValueOptions), the tool - // must surface a - // specific 'unknown field id' error including the id itself so the - // LLM can correlate parallel tool calls and recover. + // must surface a specific 'unknown field id' error including the id + // itself so the LLM can correlate parallel tool calls and recover. var registered = new TestField(); var unregistered = new TestField(); var controller = new FormAIController( @@ -323,6 +323,51 @@ void queryFieldOptionsDoesNotLeakRawExceptionContent() { + "got: " + result); } + @Test + void queryFieldOptionsRendersItemsViaExplicitItemLabelGenerator() { + // Items returned by the query callback are rendered through the + // labeler chain before the LLM sees them. An explicit labeler on + // ValueOptions wins over the field's own setItemLabelGenerator, + // so the LLM can pick from labels different from the UI ones. + var alpha = new Project("P-1", "Alpha"); + var beta = new Project("P-2", "Beta"); + var combo = new SingleSelectField(); + combo.setItemLabelGenerator(Project::name); + var controller = new FormAIController(new Div(combo)); + controller + .fieldValueOptions( + ValueOptions.forField(combo) + .options( + (filter, limit) -> List.of(alpha, beta)) + .itemLabelGenerator(Project::code), + label -> alpha); + controller.onRequest(); + + var result = executeQueryFieldOptions(controller, combo, "", 10); + + Assertions.assertEquals("P-1\nP-2\n", result, + "query_field_options labels must come from the explicit " + + "ValueOptions labeler, got: " + result); + } + + @Test + void queryFieldOptionsRendersItemsViaFieldsItemLabelGeneratorByDefault() { + // With no explicit labeler on ValueOptions, the query callback's + // items render through the field's setItemLabelGenerator. No + // duplication of the V-to-label mapping at registration time. + var alpha = new Project("P-1", "Alpha"); + var combo = new SingleSelectField(); + combo.setItemLabelGenerator(Project::name); + var controller = new FormAIController(new Div(combo)); + controller.fieldValueOptions(ValueOptions.forField(combo) + .options((filter, limit) -> List.of(alpha)), label -> alpha); + controller.onRequest(); + + var result = executeQueryFieldOptions(controller, combo, "", 10); + + Assertions.assertEquals("Alpha\n", result); + } + @Test void queryFieldOptionsSchemaIsStatic() { // The parameters schema is built once and does not enumerate field From 56bd17c88a6df21133bd70323f6a49678f6f4109 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Thu, 18 Jun 2026 14:57:29 +0300 Subject: [PATCH 2/4] chore: run formatter --- .../com/vaadin/flow/component/ai/form/FormAIControllerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormAIControllerTest.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormAIControllerTest.java index 70bda2f16f3..5cfaa069af2 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormAIControllerTest.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormAIControllerTest.java @@ -623,7 +623,7 @@ void reregisteringValueOptionsOverwritesItemLabelGenerator() { var f = json(findTool(controller.getTools(), "get_form_state") .execute(JacksonUtils.createObjectNode())).path("fields") - .get(0); + .get(0); var labels = new java.util.ArrayList(); f.path("enum").forEach(n -> labels.add(n.asString())); From e4b4feb9f63a85c03fd0ecfef6a3ad260c8dd755 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Thu, 25 Jun 2026 19:47:24 +0300 Subject: [PATCH 3/4] refactor: drop tovalue --- .../component/ai/form/FormAIController.java | 147 ++++++------- .../component/ai/form/FormFieldHints.java | 53 ++--- .../component/ai/form/FormFieldSchema.java | 6 +- .../component/ai/form/FormValueConverter.java | 123 ++++++----- .../flow/component/ai/form/ValueOptions.java | 58 ++--- .../component/ai/form/FillFormToolTest.java | 113 +++++----- .../ai/form/FormAIControllerTest.java | 150 ++++++++----- .../component/ai/form/FormStateToolTest.java | 33 +-- .../component/ai/form/FormTestFields.java | 3 +- .../ai/form/FormValueConverterTest.java | 198 +++++++++--------- .../ai/form/QueryFieldOptionsToolTest.java | 12 +- 11 files changed, 459 insertions(+), 437 deletions(-) diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormAIController.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormAIController.java index aaec26e9f4e..fc38d561be5 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormAIController.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormAIController.java @@ -18,6 +18,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; @@ -84,13 +85,11 @@ * {@code fieldValueOptions} takes a {@link ValueOptions} built via * {@link ValueOptions#forField(HasValue) forField} — the compiler picks the * {@link ValueOptions#forField(MultiSelect) MultiSelect overload} automatically - * for fields statically typed as {@link MultiSelect}. For fields whose value - * type is anything other than {@link String}, the - * {@link #fieldValueOptions(ValueOptions, Function) two-argument overload} also - * accepts a label-to-value converter, which the controller applies per label; - * for multi-select fields the resolved elements are then aggregated into a - * {@link LinkedHashSet} before {@link HasValue#setValue}. LLM-facing labels for - * the registered items are derived from the field's + * for fields statically typed as {@link MultiSelect}. The controller resolves a + * chosen label back to one of the registered items via the registration's + * item-label generator; for multi-select fields the resolved elements are + * aggregated into a {@link LinkedHashSet} before {@link HasValue#setValue}. + * LLM-facing labels are derived from the field's * {@code setItemLabelGenerator(...)} by default; see {@link ValueOptions} for * the full resolution chain. *

@@ -344,15 +343,17 @@ public FormAIController describeField(HasValue field, } /** - * Registers a known set of options for a {@link String}-typed field. Each - * item is also the label the LLM sees — the chosen label is itself the - * value written to the field, with no converter needed. For any - * non-{@link String} value type, use - * {@link #fieldValueOptions(ValueOptions, Function) the two-argument - * overload} to supply a converter; this is enforced at compile time. + * Registers a known set of items for a field. The LLM sees one label per + * item; when it picks a label, the controller walks the registration's + * items, applies the item-label generator per item, and returns the first + * whose label matches. The label-generator chain is documented on + * {@link ValueOptions}. *

- * For {@link MultiSelect MultiSelect} fields the controller wraps the - * chosen labels into a {@link LinkedHashSet} before + * Items that share a label resolve to the first in registration order; a + * fixed-options registration logs a warning when this happens. Labels that + * match no item are rejected back to the LLM with a reason it can correct + * on the next turn. For {@link MultiSelect MultiSelect} fields the resolved + * items are wrapped into a {@link LinkedHashSet} before * {@link HasValue#setValue}. Later calls for the same field overwrite * earlier ones. * @@ -361,60 +362,12 @@ public FormAIController describeField(HasValue field, * its item source set via either * {@link ValueOptions#options(Collection)} or * {@link ValueOptions#options(BiFunction)} - * @return this controller, for chaining - * @throws NullPointerException - * if {@code config} is {@code null} - * @throws IllegalArgumentException - * if the registration has no item source set; if the developer - * routed a {@code MultiSelect} field through the single-value - * {@code forField} overload (upcast reference); or if the - * field's value type is a Collection but the field does not - * implement {@link MultiSelect} - */ - public FormAIController fieldValueOptions(ValueOptions config) { - Objects.requireNonNull(config, "Value options must not be null"); - return applyValueOptions(config, Function.identity()); - } - - /** - * Registers a known set of items for a field, paired with a converter that - * resolves a chosen label to the field's value type. The controller derives - * the LLM-facing label for each item through this chain: an explicit - * {@link ValueOptions#itemLabelGenerator(ItemLabelGenerator)} if set, - * otherwise the field's own {@code setItemLabelGenerator(...)} (read - * reflectively), otherwise {@link String#valueOf(Object)} as a last resort. - * When the LLM picks a label, the controller calls {@code toValue} on it - * and writes the result to the field. If {@code toValue} returns - * {@code null} or throws — for example because the LLM picked a label the - * converter does not recognize — the write is rejected back to the LLM with - * a reason and the model can correct on the next turn. - *

- * A typical converter delegates to a service or repository that looks the - * domain object up by its display name, for example - * {@code label -> projectService.findByName(label)} for a - * {@code ComboBox}. For {@link MultiSelect MultiSelect} fields the - * converter runs once per chosen label and the controller wraps the - * resolved elements into a {@link LinkedHashSet} before - * {@link HasValue#setValue}. - *

- * Later calls for the same field overwrite earlier ones. Use - * {@link #fieldValueOptions(ValueOptions) the single-argument overload} for - * {@link String}-typed fields; the converter is implicit there. - * - * @param config - * the field's options registration, not {@code null}; must have - * its item source set via either - * {@link ValueOptions#options(Collection)} or - * {@link ValueOptions#options(BiFunction)} - * @param toValue - * converts a chosen label to one element of the field's value - * type, not {@code null} * @param * the item type — the field's value type for single-value * fields, the per-element type for multi-select * @return this controller, for chaining * @throws NullPointerException - * if {@code config} or {@code toValue} is {@code null} + * if {@code config} is {@code null} * @throws IllegalArgumentException * if the registration has no item source set; if the developer * routed a {@code MultiSelect} field through the single-value @@ -422,15 +375,12 @@ public FormAIController fieldValueOptions(ValueOptions config) { * field's value type is a Collection but the field does not * implement {@link MultiSelect} */ - public FormAIController fieldValueOptions(ValueOptions config, - Function toValue) { + public FormAIController fieldValueOptions(ValueOptions config) { Objects.requireNonNull(config, "Value options must not be null"); - Objects.requireNonNull(toValue, "Value converter must not be null"); - return applyValueOptions(config, toValue); + return applyValueOptions(config); } - private FormAIController applyValueOptions(ValueOptions config, - Function toValue) { + private FormAIController applyValueOptions(ValueOptions config) { var fixed = config.fixedOptions(); var query = config.query(); if ((fixed == null) == (query == null)) { @@ -438,22 +388,18 @@ private FormAIController applyValueOptions(ValueOptions config, "ValueOptions requires options(...) " + "(fixed Collection or query BiFunction)"); } - // The single-value forField overload accepts MultiSelect fields whose - // static reference is upcast to HasValue. Reject so the typed - // MultiSelect overload remains the only path for multi-select - // registrations and toValue stays per-item rather than Set-returning. + // forField(HasValue) accepts MultiSelect fields whose static reference + // is upcast. Reject so the typed MultiSelect overload is the only + // entry for multi-select registrations. var isMultiSelect = config.field() instanceof MultiSelect; if (!config.isMulti() && isMultiSelect) { throw new IllegalArgumentException( "Field implements MultiSelect — declare the reference as " + "MultiSelect so the MultiSelect-typed forField " - + "overload picks up, and toValue can return one " - + "item rather than a Set"); + + "overload picks up"); } - // Collection-valued fields must implement MultiSelect — otherwise we - // have no defined aggregation for per-label converter results. - // field.getEmptyValue() is the runtime signal: non-MultiSelect fields - // whose empty value is a Collection are the case we reject. + // Collection-valued fields must implement MultiSelect — otherwise + // there is no defined aggregation for the resolved items. if (!isMultiSelect && config.field().getEmptyValue() instanceof Collection) { throw new IllegalArgumentException( @@ -465,24 +411,59 @@ private FormAIController applyValueOptions(ValueOptions config, var labeler = resolveItemLabeler(config.field(), config.itemLabelGenerator()); var hints = hintsFor(config.field()); - hints.valueOptionsToValue = toValue; hints.itemLabelGenerator = labeler; + // Items the converter resolves chosen labels against. Pre-populated + // with the fixed list, or appended on each query-callback invocation. + var observedItems = new ArrayList<>(); + hints.valueOptionsItems = observedItems; if (fixed != null) { @SuppressWarnings({ "unchecked", "rawtypes" }) List items = (List) fixed; + observedItems.addAll(items); + warnOnDuplicateLabels(items, labeler); hints.valueOptionsQuery = (filter, limit) -> filterAndLimit(items, filter, limit, labeler); hints.fixedOptions = true; } else { @SuppressWarnings({ "unchecked", "rawtypes" }) BiFunction> rawQuery = (BiFunction) query; - hints.valueOptionsQuery = (filter, limit) -> rawQuery - .apply(filter, limit).stream().map(labeler).toList(); + hints.valueOptionsQuery = (filter, limit) -> { + var batch = rawQuery.apply(filter, limit); + observedItems.addAll(batch); + return batch.stream().map(labeler).toList(); + }; hints.fixedOptions = false; } return this; } + /** + * Logs a warning when two or more items in a fixed-options registration + * render to the same label. Resolution falls back to first-in-list + * ordering, so duplicates are recoverable but ambiguous — a unique + * {@link ValueOptions#itemLabelGenerator(ItemLabelGenerator)} is the + * unambiguous fix. + */ + private static void warnOnDuplicateLabels(List items, + Function labeler) { + var seen = new HashSet(); + var duplicates = new LinkedHashSet(); + for (var item : items) { + var label = labeler.apply(item); + if (!seen.add(label)) { + duplicates.add(label); + } + } + if (!duplicates.isEmpty()) { + LOGGER.warn( + "ValueOptions registration contains items with duplicate " + + "labels {}; the first item per label will win on " + + "resolution. Supply a unique itemLabelGenerator " + + "to disambiguate.", + duplicates); + } + } + /** * Resolves the V-to-label function for one valueOptions registration. * Priority: an explicit {@link ItemLabelGenerator} on the registration, diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldHints.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldHints.java index 3840784f9be..df0a06c7f90 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldHints.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldHints.java @@ -21,45 +21,50 @@ /** * Mutable per-field hint state held by {@link FormAIController}, keyed by the - * field's opaque id. - *

- * Set by {@link FormAIController#fieldValueOptions(ValueOptions) - * controller.fieldValueOptions(...)}: {@link #valueOptionsQuery} returns the - * labels the LLM sees for a given filter (the controller wraps the - * {@link ValueOptions}' item source + label generator into this single - * label-producing callback at registration time), {@link #fixedOptions} flags - * whether the schema should render the options as {@code enum} or - * {@code queryable}, {@link #valueOptionsToValue} resolves one label back to - * one element, and {@link #itemLabelGenerator} renders the field's current - * value through the same labeler so the value string matches the labels the LLM - * was offered. For multi-select fields the controller wraps the resolved - * elements into a {@link java.util.LinkedHashSet} before {@code setValue}; the - * hint state is the same shape in both cases. + * field's opaque id. Populated by {@code describeField}, {@code ignoreField}, + * and {@code fieldValueOptions}; consumed by {@link FormFieldSchema} when + * building the {@code get_form_state} payload and by {@link FormValueConverter} + * when applying {@code fill_form} values. * * @author Vaadin Ltd */ final class FormFieldHints { String description; + /** + * Label-producing callback the {@code query_field_options} tool drives. + * Wraps the {@link ValueOptions} item source plus + * {@link #itemLabelGenerator} into a single (filter, limit) → labels + * function. Non-{@code null} whenever {@code fieldValueOptions} has been + * called for this field. + */ BiFunction> valueOptionsQuery; - Function valueOptionsToValue; /** - * Item-to-label function used to render the current value when - * value-options is registered. Resolved at registration to the explicit + * Items the controller has seen for this registration: the fixed list for + * {@link ValueOptions#options(java.util.Collection)}, or items accumulated + * from {@link ValueOptions#options(java.util.function.BiFunction)} batches. + * {@link FormValueConverter} walks this list at fill time, applies + * {@link #itemLabelGenerator} per item, and returns the first whose label + * matches the LLM-supplied one (insertion order — first-wins on + * duplicates). Non-{@code null} whenever {@link #valueOptionsQuery} is set; + * empty until the query callback runs for query-mode registrations. + */ + List valueOptionsItems; + /** + * Item-to-label function used to render the field's current value and to + * resolve LLM-supplied labels back to items via {@link #valueOptionsItems}. + * Resolved at registration to the explicit * {@link ValueOptions#itemLabelGenerator(com.vaadin.flow.component.ItemLabelGenerator)} * or to a delegate that defers to {@link FormValueConverter#renderItem} * (field's own {@code getItemLabelGenerator()}, then * {@link String#valueOf(Object)}). Non-{@code null} whenever - * {@link #valueOptionsQuery} is set, so the schema's value string agrees - * with the labels emitted in the {@code enum} / {@code query_field_options} - * payloads. + * {@link #valueOptionsQuery} is set. */ Function itemLabelGenerator; /** - * {@code true} when the field was registered with the fixed-options - * variant; {@code false} when registered with a query callback or with no - * value-options hint at all. Used by {@link FormFieldSchema} to choose - * {@code enum} vs {@code queryable} in the {@code get_form_state} JSON. + * {@code true} for fixed-options registrations, {@code false} for + * query-callback or no-value-options registrations. Drives the {@code enum} + * vs {@code queryable} choice in {@link FormFieldSchema}. */ boolean fixedOptions; boolean ignored; diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldSchema.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldSchema.java index 47001c77bcd..41c74158738 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldSchema.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldSchema.java @@ -100,9 +100,9 @@ private static void applyType(ObjectNode node, HasValue field, return; } // fieldValueOptions turns any field into a constrained-choice field - // from the LLM's perspective: the LLM picks a label, - // valueOptionsToValue converts back. Emit type=string + enum/queryable - // so the LLM sees the signal regardless of the underlying value type. + // from the LLM's perspective: the LLM picks a label, the controller + // resolves it to an item. Emit type=string + enum/queryable so the + // LLM sees the signal regardless of the underlying value type. if (type == FormFieldType.SINGLE_SELECT || hasValueOptions(hints)) { node.put(FIELD_TYPE, TYPE_STRING); applySelectionOptions(node, field, hints); diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormValueConverter.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormValueConverter.java index ba9074175ca..2fb61aa6e5d 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormValueConverter.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormValueConverter.java @@ -28,7 +28,6 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; -import java.util.function.Function; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -178,34 +177,27 @@ static final class RejectedValueException extends RuntimeException { /** * Converts a JSON node into a typed value suitable for - * {@link HasValue#setValue}. Returns the field's empty value when the JSON - * node is {@code null} or a JSON {@code null}. Throws - * {@link RejectedValueException} when the JSON shape doesn't match the - * field's type, or when the registered {@code fieldValueOptions} cannot - * resolve a label. + * {@link HasValue#setValue}. Returns the field's empty value for + * {@code null} or JSON {@code null}; throws {@link RejectedValueException} + * when the JSON shape doesn't match the field's type, or when a registered + * {@code fieldValueOptions} cannot resolve a label. *

- * Selection and {@code fieldValueOptions} routing: when a field has - * a {@code valueOptionsToValue} registered, the LLM sees the field as an - * {@code enum} or {@code queryable} string in {@code get_form_state} and - * picks one or more labels. The label-to-value resolution happens here so - * the resulting object matches the field's value type before - * {@link HasValue#setValue} sees it. Multi-select fields expect a JSON - * array of labels; single-select and other label-routed fields expect a - * single string. A selection field without a {@code fieldValueOptions} - * registration falls back to matching the supplied label(s) against the - * field's own in-memory items (an eager {@code setItems(...)}), which carry - * the same labels {@code get_form_state} advertised; only a field that has - * neither a {@code fieldValueOptions} registration nor items is rejected - * with a curated reason naming the missing registration. + * For selection fields, resolution tries the registration's observed items + * first (matching the LLM-supplied label against + * {@link FormFieldHints#itemLabelGenerator} per item), then falls back to + * the field's own in-memory items (an eager {@code setItems(...)}). A + * selection field with no item source is rejected with a curated reason + * naming the missing registration. Multi-select fields expect a JSON array + * of labels; single-select and other label-routed fields expect a single + * string. */ static Object convert(FormFieldDescriptor field, JsonNode value) { if (value == null || value.isNull()) { return field.field().getEmptyValue(); } - // Multi-select takes an array of labels and applies toValue per - // element; handled before the fieldValueOptions check so the array - // shape is enforced even on fields whose value type is itself a String - // set. + // Multi-select takes an array of labels; handled before the + // fieldValueOptions check so the array shape is enforced even on + // fields whose value type is itself a String set. if (field.type() == FormFieldType.MULTI_SELECT) { return convertMultiSelect(field, value); } @@ -214,8 +206,8 @@ static Object convert(FormFieldDescriptor field, JsonNode value) { // parsing would hand setValue a raw String and the field would reject // it. var hints = field.hints(); - if (hints != null && hints.valueOptionsToValue != null) { - return convertSingleLabel(value, hints.valueOptionsToValue); + if (hints != null && hints.valueOptionsItems != null) { + return convertSingleLabelFromObservedItems(value, hints); } return switch (field.type()) { case STRING, EMAIL -> convertString(value); @@ -262,13 +254,21 @@ private static Object convertSingleSelectFromItems( return resolveLabelAgainstItems(field.field(), value.asString(), items); } - private static Object convertSingleLabel(JsonNode value, - Function toValue) { + /** + * Resolves one LLM-supplied label against the items the registration has + * seen. Walks {@link FormFieldHints#valueOptionsItems} and returns the + * first item whose label (via {@link FormFieldHints#itemLabelGenerator}) + * matches. An empty observed-items list is the query-mode "registered but + * never queried" case and produces a rejection that nudges the LLM to call + * {@code query_field_options} first. + */ + private static Object convertSingleLabelFromObservedItems(JsonNode value, + FormFieldHints hints) { if (!value.isString()) { throw new RejectedValueException( "Expected string label, got " + value); } - return resolveLabel(value.asString(), toValue); + return resolveLabelAgainstObservedItems(value.asString(), hints); } private static Object convertMultiSelect(FormFieldDescriptor field, @@ -284,19 +284,42 @@ private static Object convertMultiSelect(FormFieldDescriptor field, return field.field().getEmptyValue(); } var hints = field.hints(); - var toValue = hints != null ? hints.valueOptionsToValue : null; - if (toValue == null) { - return convertMultiSelectFromItems(field, value); + if (hints != null && hints.valueOptionsItems != null) { + var result = new LinkedHashSet<>(); + for (var node : value) { + if (!node.isString()) { + throw new RejectedValueException( + "Expected string label, got " + node); + } + result.add(resolveLabelAgainstObservedItems(node.asString(), + hints)); + } + return result; } - var result = new LinkedHashSet<>(); - for (var node : value) { - if (!node.isString()) { - throw new RejectedValueException( - "Expected string label, got " + node); + return convertMultiSelectFromItems(field, value); + } + + /** + * Walks {@link FormFieldHints#valueOptionsItems} and returns the first item + * whose label (via {@link FormFieldHints#itemLabelGenerator}) matches the + * LLM-supplied one. An empty list is called out with a hint at + * {@code query_field_options}, since that's the only way the LLM could have + * reached this point without seeing options. + */ + private static Object resolveLabelAgainstObservedItems(String label, + FormFieldHints hints) { + for (var item : hints.valueOptionsItems) { + if (label.equals(hints.itemLabelGenerator.apply(item))) { + return item; } - result.add(resolveLabel(node.asString(), toValue)); } - return result; + if (hints.valueOptionsItems.isEmpty()) { + throw new RejectedValueException("No matching option for label: " + + label + + " (call query_field_options first to load the field's options)"); + } + throw new RejectedValueException( + "No matching option for label: " + label); } /** @@ -346,30 +369,6 @@ private static Object resolveLabelAgainstItems(HasValue field, "No matching option for label: " + label); } - /** - * Resolves one LLM-supplied label via the application's - * {@code valueOptionsToValue}. Wraps the application's throw and the - * application's {@code null} return into curated rejection reasons — both - * surface back to the LLM verbatim through the {@code rejected} block, so - * the message must not leak third-party detail. - */ - private static Object resolveLabel(String label, - Function toValue) { - Object resolved; - try { - resolved = toValue.apply(label); - } catch (RuntimeException ex) { - LOGGER.debug("valueOptionsToValue threw for label {}", label, ex); - throw new RejectedValueException( - "Could not resolve label '" + label + "' to a value."); - } - if (resolved == null) { - throw new RejectedValueException( - "No matching option for label: " + label); - } - return resolved; - } - private static String convertString(JsonNode value) { if (!value.isString()) { throw new RejectedValueException("Expected string, got " + value); diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/ValueOptions.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/ValueOptions.java index 68a452c8ea8..e508f8b6925 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/ValueOptions.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/ValueOptions.java @@ -20,7 +20,6 @@ import java.util.List; import java.util.Objects; import java.util.function.BiFunction; -import java.util.function.Function; import com.vaadin.flow.component.Component; import com.vaadin.flow.component.HasValue; @@ -28,28 +27,22 @@ import com.vaadin.flow.data.selection.MultiSelect; /** - * Per-field registration of the items the LLM may pick from for a given field. - * The item set is either fixed ({@link #options(Collection)}) or supplied on - * demand by a callback ({@link #options(BiFunction)}); exactly one of the two - * must be set, with the last call winning. Pass the configured registration to - * {@link FormAIController#fieldValueOptions(ValueOptions) - * controller.fieldValueOptions(...)} to apply it; the items are then rendered - * to labels and presented to the LLM as the field's choices. - *

- * Items carry the field's value type, so a {@code ComboBox} - * registration passes {@code Project} items, not pre-rendered label strings. - * The controller derives the LLM-facing label for each item through this chain: - * an explicit {@link #itemLabelGenerator(ItemLabelGenerator)} if set, otherwise - * the field's own {@code setItemLabelGenerator(...)} (read reflectively), - * otherwise {@link String#valueOf(Object)} as a last resort. The label produced - * by this chain is what the LLM sees, picks from, and sends back to the - * {@code toValue} converter — so the field's UI label generator drives the AI - * labels automatically when set. + * Per-field registration of the items the LLM may pick from. Items carry the + * field's value type — a {@code ComboBox} registration passes + * {@code Project} items, not pre-rendered strings. Pass the configured + * registration to {@link FormAIController#fieldValueOptions(ValueOptions) + * controller.fieldValueOptions(...)} to apply it. *

- * For a non-{@link String} field, supply a label-to-value converter through - * {@link FormAIController#fieldValueOptions(ValueOptions, Function) - * fieldValueOptions(config, toValue)} — the converter resolves a chosen label - * back to the field's value type before the controller writes it. + * The item set is either fixed ({@link #options(Collection)}) or supplied on + * demand by a callback ({@link #options(BiFunction)}); exactly one must be set, + * with the last call winning. Each item is rendered to an LLM-facing label + * through this chain: the explicit + * {@link #itemLabelGenerator(ItemLabelGenerator)} if set, otherwise the field's + * own {@code setItemLabelGenerator(...)} (read reflectively), otherwise + * {@link String#valueOf(Object)}. The controller resolves a chosen label back + * to an item by applying the same chain to each registered item and returning + * the first whose label matches, so the field's UI label generator drives the + * AI labels automatically. * * @param * the item type — the field's value type for single-value fields, @@ -72,11 +65,7 @@ private ValueOptions(HasValue field, boolean multi) { /** * Starts an options registration for a single-value field. The field's - * value type {@code V} flows through; for any {@code V} other than - * {@link String}, the controller's two-argument - * {@link FormAIController#fieldValueOptions(ValueOptions, Function)} - * overload must be used to supply a label-to-value converter (a - * compile-time requirement, not a runtime check). + * value type {@code V} flows through unchanged. * * @param field * the single-value field whose options the LLM may pick from, @@ -95,11 +84,8 @@ public static ValueOptions forField(HasValue field) { * Starts an options registration for a multi-select field. Picked by the * compiler over {@link #forField(HasValue)} whenever the reference is * statically typed as {@link MultiSelect}. The per-element type {@code T} - * flows through; for any {@code T} other than {@link String}, the - * controller's two-argument - * {@link FormAIController#fieldValueOptions(ValueOptions, Function)} - * overload must be used. The controller aggregates resolved per-label items - * into a {@link LinkedHashSet} before {@link HasValue#setValue}. + * flows through unchanged; resolved items are aggregated into a + * {@link LinkedHashSet} before {@link HasValue#setValue}. * * @param field * the multi-select field whose options the LLM may pick from, @@ -176,10 +162,10 @@ public ValueOptions options(BiFunction> query) { * needs a different label from the UI (for example a code rather than a * display name). Calling this multiple times overwrites the previous value. *

- * Two items rendering to the same label are not an error: the duplicate - * appears verbatim in the LLM's option list, and when the LLM picks the - * shared label the supplied {@code toValue} converter decides which item is - * written. Emit unique labels per item if disambiguation matters. + * Two items rendering to the same label are resolvable but ambiguous: + * resolution picks the first matching item in registration order, and the + * controller logs a warning at registration when this happens. Emit unique + * labels per item if disambiguation matters. * * @param generator * the per-item label generator, not {@code null} diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FillFormToolTest.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FillFormToolTest.java index dfdb8f2709f..5cab42cabc1 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FillFormToolTest.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FillFormToolTest.java @@ -15,6 +15,7 @@ */ package com.vaadin.flow.component.ai.form; +import static com.vaadin.flow.component.ai.form.FormTestSupport.executeQueryFieldOptions; import static com.vaadin.flow.component.ai.form.FormTestSupport.findTool; import static com.vaadin.flow.component.ai.form.FormTestSupport.idOf; @@ -24,7 +25,6 @@ import java.time.LocalTime; import java.util.ArrayList; import java.util.List; -import java.util.Map; import java.util.Set; import org.junit.jupiter.api.Assertions; @@ -1288,33 +1288,50 @@ void fillForm_callbackThrowingRuntimeException_returnsGenericError() { } @Test - void fillForm_singleSelect_writesResolvedValueViaValueOptionsToValue() { - // The LLM speaks in labels for SINGLE_SELECT fields with - // fieldValueOptions registered. The tool must apply toValue so the - // field gets the domain instance, not the raw label string. + void fillForm_singleSelect_stringValueWithCustomLabeler_writesOriginalItem() { + // V=String with a custom ValueOptions.itemLabelGenerator: the LLM + // is shown short codes; the field must receive the original full + // string, not the code the LLM sent back. + var field = new SingleSelectField(); + var controller = newController(field); + controller.fieldValueOptions(ValueOptions.forField(field) + .options(List.of("Premium", "Basic")) + .itemLabelGenerator(s -> s.substring(0, 1))); + controller.onRequest(); + + var result = fillFormResult(controller, payload(field, "\"P\"")); + + Assertions.assertEquals("Premium", field.getValue(), + "Field must receive the original item, not the labeled " + + "short code"); + Assertions.assertTrue(success(result)); + } + + @Test + void fillForm_singleSelect_writesResolvedValueViaValueOptions() { + // The field must receive the original domain item that the labeler + // renders to the LLM-supplied label, not the raw label string. var apollo = new Project("P-1", "Apollo"); - var projects = Map.of("Apollo", apollo); var field = new SingleSelectField(); field.setItemLabelGenerator(Project::name); var controller = newController(field); - controller.fieldValueOptions(ValueOptions.forField(field) - .options((filter, limit) -> List.of(apollo)), projects::get); + controller.fieldValueOptions( + ValueOptions.forField(field).options(List.of(apollo))); controller.onRequest(); var result = fillFormResult(controller, payload(field, "\"Apollo\"")); Assertions.assertEquals(apollo, field.getValue(), - "Field must receive the resolved domain object, not the " - + "raw label string"); + "Field must receive the registered domain object"); Assertions.assertTrue(success(result)); } @Test void fillForm_singleSelect_withoutValueOptionsIsRejectedWithHint() { - // SINGLE_SELECT without a fieldValueOptions registration means the - // LLM has no labels to pick and the converter has no toValue to - // resolve. The fill must fail loudly with a reason that points - // at the missing registration so the developer can fix it. + // SINGLE_SELECT without a fieldValueOptions registration and no + // eager setItems(...) has no option source at all. The fill must + // fail loudly with a reason that points at the missing registration + // so the developer can fix it. var field = new SingleSelectField(); var controller = controllerFor(field); @@ -1395,17 +1412,15 @@ void fillForm_singleSelect_eagerItemsResolveLabelViaItemLabelGenerator() { @Test void fillForm_singleSelect_unknownLabelIsRejected() { - // toValue returning null is the agreed signal for "label doesn't - // match any option". The orchestrator must reject rather than - // pass null to setValue (which would silently clear the field). + // A label that matches no registered item must reject rather than + // silently clear the field; the reason names the offending label so + // the LLM can self-correct. + var apollo = new Project("P-1", "Apollo"); var field = new SingleSelectField(); field.setItemLabelGenerator(Project::name); var controller = newController(field); controller.fieldValueOptions( - ValueOptions.forField(field) - .options((filter, limit) -> List - .of(new Project("P-1", "Apollo"))), - label -> null); + ValueOptions.forField(field).options(List.of(apollo))); controller.onRequest(); var result = fillFormResult(controller, payload(field, "\"Unknown\"")); @@ -1421,18 +1436,16 @@ void fillForm_singleSelect_unknownLabelIsRejected() { @Test void fillForm_multiSelect_writesResolvedSetViaValueOptions() { - // fieldValueOptions on a MultiSelectField takes a single-item Function; - // the converter accumulates per-label items into the Set - // value type via an internal LinkedHashSet. - var apollo = new Project("Apollo", "Apollo"); - var vega = new Project("Vega", "Vega"); + // The converter resolves each LLM-supplied label against the + // registered items and aggregates the matches into the field's + // Set value type. + var apollo = new Project("APL", "Apollo"); + var vega = new Project("VGA", "Vega"); var field = new MultiSelectField(); field.setItemLabelGenerator(Project::name); var controller = newController(field); controller.fieldValueOptions( - ValueOptions.forField(field) - .options((filter, limit) -> List.of(apollo, vega)), - label -> new Project(label, label)); + ValueOptions.forField(field).options(List.of(apollo, vega))); controller.onRequest(); var result = fillFormResult(controller, @@ -1523,15 +1536,15 @@ void fillForm_multiSelect_eagerItemsResolveLabelsViaItemLabelGenerator() { @Test void fillForm_multiSelect_emptyArrayClearsField() { // Empty array is the LLM clearing the multi-select; the converter - // builds an empty LinkedHashSet so the field's setValue receives - // an empty Set. + // short-circuits to the field's empty value without consulting the + // items list. var field = new MultiSelectField(); field.setItemLabelGenerator(Project::name); var existing = new Project("X", "X"); field.setValue(Set.of(existing)); var controller = newController(field); controller.fieldValueOptions(ValueOptions.forField(field) - .options((filter, limit) -> List.of()), label -> existing); + .options((filter, limit) -> List.of(existing))); controller.onRequest(); var result = fillFormResult(controller, payload(field, "[]")); @@ -1545,16 +1558,14 @@ void fillForm_multiSelect_nonArrayPayloadIsRejected() { // The LLM sends a bare string for a multi-select field. The // converter enforces the array shape so a stray scalar payload // doesn't reach setValue. - var apollo = new Project("Apollo", "Apollo"); + var apollo = new Project("APL", "Apollo"); var field = new MultiSelectField(); field.setItemLabelGenerator(Project::name); var existing = new Project("X", "X"); field.setValue(Set.of(existing)); var controller = newController(field); - controller.fieldValueOptions( - ValueOptions.forField(field) - .options((filter, limit) -> List.of(apollo)), - label -> new Project(label, label)); + controller.fieldValueOptions(ValueOptions.forField(field) + .options((filter, limit) -> List.of(apollo))); controller.onRequest(); var result = fillFormResult(controller, payload(field, "\"Apollo\"")); @@ -1571,43 +1582,39 @@ void fillForm_multiSelect_nonArrayPayloadIsRejected() { @Test void fillForm_multiSelect_reregistrationOverwritesPriorOptions() { // fieldValueOptions called twice on the same MultiSelect field — the - // second call wins, including switching from fixed to queryable - // and replacing the toValue function. + // second call wins, including switching from fixed to queryable. var first = new Project("v1", "First"); var second = new Project("v2", "Second"); var field = new MultiSelectField(); field.setItemLabelGenerator(Project::name); var controller = newController(field); controller.fieldValueOptions( - ValueOptions.forField(field).options(List.of(first)), - label -> new Project("v1", label)); - controller.fieldValueOptions( - ValueOptions.forField(field) - .options((filter, limit) -> List.of(second)), - label -> new Project("v2", label)); + ValueOptions.forField(field).options(List.of(first))); + controller.fieldValueOptions(ValueOptions.forField(field) + .options((filter, limit) -> List.of(second))); controller.onRequest(); + executeQueryFieldOptions(controller, field, "", 10); var result = fillFormResult(controller, payload(field, "[\"Second\"]")); Assertions.assertTrue(success(result)); Assertions.assertEquals(Set.of(second), field.getValue(), - "Re-registration must hand the LLM-supplied label to the " - + "second toValue, not the first"); + "Re-registration must resolve against the second " + + "registration's items, not the first"); } @Test - void fillForm_fieldValueOptionsOnPrimitiveTypeUsesToValueNotTypeDrivenParsing() { + void fillForm_fieldValueOptionsOnPrimitiveTypeSkipsTypeDrivenParsing() { // Registering fieldValueOptions(...) on a non-SELECT field still makes // the LLM speak in labels (the schema advertises an enum/queryable - // string). The converter must apply toValue and skip type-driven - // parsing — otherwise the field would receive a raw String and a - // typed setValue (e.g. ComboBox) would reject it. + // string). The converter must resolve via the items list and skip + // type-driven parsing — otherwise the field would receive a raw + // String and the typed setValue would reject it. var field = new IntField(); var controller = newController(field); controller.fieldValueOptions( ValueOptions.forField(field).options(List.of(1, 10)) - .itemLabelGenerator(v -> v == 1 ? "low" : "high"), - label -> "low".equals(label) ? 1 : 10); + .itemLabelGenerator(v -> v == 1 ? "low" : "high")); controller.onRequest(); var result = fillFormResult(controller, payload(field, "\"high\"")); diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormAIControllerTest.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormAIControllerTest.java index 5cfaa069af2..ba7d6c93ca9 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormAIControllerTest.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormAIControllerTest.java @@ -516,37 +516,27 @@ void hintMethodsRejectNullPayload() { () -> config.options((java.util.Collection) null)); Assertions.assertThrows(NullPointerException.class, () -> config.itemLabelGenerator(null)); - // A null converter passed to the controller's two-argument - // overload throws NPE. - Assertions.assertThrows(NullPointerException.class, - () -> controller.fieldValueOptions(ValueOptions - .forField(new IntField()).options(List.of(1)), - null)); // An empty fixed-options list leaves the field un-fillable; // rejected at the options() call site. Assertions.assertThrows(IllegalArgumentException.class, () -> config.options(List. of())); - // A ValueOptions with no options(...) set is rejected by the - // controller at registration. The missing-converter case is - // compile-time enforced (no overload accepts a non-String - // ValueOptions without a Function argument), so it has no - // corresponding runtime test. + // A ValueOptions with no options(...) set is rejected at + // registration. Assertions.assertThrows(IllegalArgumentException.class, () -> controller .fieldValueOptions(ValueOptions.forField(field))); } @Test - void fieldValueOptionsAcceptsNonStringFieldWithToValueConverter() { + void fieldValueOptionsAcceptsAnyValueType() { // fieldValueOptions is generic over the field's value type — an // Integer-valued field carries Integer items, and the - // String.valueOf fallback renders the LLM-facing labels when - // the field has no item-label generator of its own. + // String.valueOf fallback renders the LLM-facing labels when the + // field has no item-label generator of its own. var field = new IntField(); var controller = new FormAIController(new Div(field)); controller.fieldValueOptions( - ValueOptions.forField(field).options(List.of(1, 2, 3)), - Integer::parseInt); + ValueOptions.forField(field).options(List.of(1, 2, 3))); controller.onRequest(); Assertions.assertEquals("1\n2\n3\n", @@ -554,9 +544,7 @@ void fieldValueOptionsAcceptsNonStringFieldWithToValueConverter() { } @Test - void identityToValueLetsStringFieldsUseLabelsDirectly() { - // Passing Function.identity() for toValue is the canonical - // String-field shortcut: the chosen label is the value as-is. + void fixedOptionsExposeLabelsThroughQueryTool() { // Smoke-test both shapes (query callback and fixed collection). var queriedField = new TestField(); var fixedField = new TestField(); @@ -614,12 +602,10 @@ void reregisteringValueOptionsOverwritesItemLabelGenerator() { var controller = new FormAIController(new Div(combo)); controller.fieldValueOptions( ValueOptions.forField(combo).options(List.of(alpha)) - .itemLabelGenerator(FormTestFields.Project::code), - label -> alpha); + .itemLabelGenerator(FormTestFields.Project::code)); controller.fieldValueOptions( ValueOptions.forField(combo).options(List.of(alpha)) - .itemLabelGenerator(FormTestFields.Project::name), - label -> alpha); + .itemLabelGenerator(FormTestFields.Project::name)); var f = json(findTool(controller.getTools(), "get_form_state") .execute(JacksonUtils.createObjectNode())).path("fields") @@ -638,20 +624,18 @@ void reregisteringValueOptionsOverwritesItemLabelGenerator() { @Test void fieldValueOptionsForFieldOnUpcastMultiSelectReferenceThrowsIllegalArgument() { // A MultiSelect statically typed as such picks the MultiSelect- - // typed forField overload at compile time. This runtime check - // is for the upcast case: the developer holds a HasValue - // reference to a MultiSelect instance, so the compiler picks - // the single-value overload and the controller would otherwise - // accept a Set-returning converter. The check redirects to the - // typed MultiSelect overload. + // typed forField overload at compile time. This runtime check is + // for the upcast case: the developer holds a HasValue reference + // to a MultiSelect instance, so the compiler picks the + // single-value overload. The check redirects to the typed + // MultiSelect overload. var multiSelect = new FormTestFields.MultiSelectField(); HasValue> upcast = multiSelect; var controller = new FormAIController(new Div(multiSelect)); var ex = Assertions.assertThrows(IllegalArgumentException.class, - () -> controller.fieldValueOptions( - ValueOptions.forField(upcast) - .options(List.of(java.util.Set.of("a"))), - label -> java.util.Set.of(label))); + () -> controller + .fieldValueOptions(ValueOptions.forField(upcast) + .options(List.of(java.util.Set.of("a"))))); Assertions.assertTrue(ex.getMessage().contains("MultiSelect"), "Rejection must name MultiSelect so the developer can " + "tighten the reference type; got: " @@ -661,18 +645,13 @@ void fieldValueOptionsForFieldOnUpcastMultiSelectReferenceThrowsIllegalArgument( @Test void fieldValueOptionsRejectsCollectionValuedFieldNotImplementingMultiSelect() { // Collection-valued fields must implement MultiSelect; otherwise - // there is no defined aggregation for per-label converter results - // and the controller refuses to register them. + // there is no defined aggregation for the resolved items and the + // controller refuses to register them. var field = new FormTestFields.CollectionWithoutMultiSelectField(); var controller = new FormAIController(new Div(field)); - var ex = Assertions - .assertThrows(IllegalArgumentException.class, - () -> controller - .fieldValueOptions( - ValueOptions.forField(field) - .options(List - .of(List.of("a"))), - label -> List.of(label))); + var ex = Assertions.assertThrows(IllegalArgumentException.class, + () -> controller.fieldValueOptions(ValueOptions + .forField(field).options(List.of(List.of("a"))))); Assertions.assertTrue( ex.getMessage().contains("Collection") && ex.getMessage().contains("MultiSelect"), @@ -682,16 +661,14 @@ void fieldValueOptionsRejectsCollectionValuedFieldNotImplementingMultiSelect() { } @Test - void fieldValueOptionsAcceptsTypedMultiSelectFieldWithNonStringConverter() { + void fieldValueOptionsAcceptsTypedMultiSelectFieldWithNonStringElementType() { // Counterpart to the Collection-value rejection: a MultiSelect- - // typed field with a non-String per-element type and an explicit - // converter must remain accepted, even though its empty value is - // itself a Collection. + // typed field with a non-String per-element type must remain + // accepted, even though its empty value is itself a Collection. var field = new FormTestFields.MultiSelectField(); var controller = new FormAIController(new Div(field)); Assertions.assertDoesNotThrow(() -> controller.fieldValueOptions( - ValueOptions.forField(field).options(List.of(1, 2)), - Integer::parseInt)); + ValueOptions.forField(field).options(List.of(1, 2)))); } @Test @@ -756,6 +733,81 @@ void multiSelectFixedOptionsRenderAsItemsEnum() { + "not queryable; got items: " + items); } + @Test + void fixedOptionsWithDuplicateLabelsWarnsAtRegistration() { + // Resolution under duplicate labels falls back to first-in-list + // ordering; the developer needs a registration-time signal so + // the ambiguity is fixed before the LLM sees it. + TestLoggerFactory.getTestLogger(FormAIController.class).clearAll(); + var first = new FormTestFields.Project("P-1", "Apollo"); + var dup = new FormTestFields.Project("P-2", "Apollo"); + var combo = new SingleSelectField(); + combo.setItemLabelGenerator(FormTestFields.Project::name); + var controller = new FormAIController(new Div(combo)); + + controller.fieldValueOptions( + ValueOptions.forField(combo).options(List.of(first, dup))); + + var warnings = TestLoggerFactory + .getTestLogger(FormAIController.class).getLoggingEvents() + .stream().filter(e -> e.getLevel() == Level.WARN).toList(); + Assertions.assertEquals(1, warnings.size(), + "Duplicate-label registration must log exactly one " + + "warning; got: " + warnings); + var formatted = warnings.get(0).getFormattedMessage(); + Assertions.assertTrue(formatted.contains("Apollo"), + "Warning must name the offending label so the developer " + + "can locate it; got: " + formatted); + Assertions.assertTrue(formatted.contains("itemLabelGenerator"), + "Warning must point at itemLabelGenerator as the " + + "disambiguation knob; got: " + formatted); + } + + @Test + void fixedOptionsWithUniqueLabelsDoesNotWarn() { + // Negative guard so the warning doesn't flood logs in the + // common unique-labels case. + TestLoggerFactory.getTestLogger(FormAIController.class).clearAll(); + var apollo = new FormTestFields.Project("P-1", "Apollo"); + var vega = new FormTestFields.Project("P-2", "Vega"); + var combo = new SingleSelectField(); + combo.setItemLabelGenerator(FormTestFields.Project::name); + var controller = new FormAIController(new Div(combo)); + + controller.fieldValueOptions(ValueOptions.forField(combo) + .options(List.of(apollo, vega))); + + var warnings = TestLoggerFactory + .getTestLogger(FormAIController.class).getLoggingEvents() + .stream().filter(e -> e.getLevel() == Level.WARN).toList(); + Assertions.assertTrue(warnings.isEmpty(), + "Unique-label registration must not warn; got: " + + warnings); + } + + @Test + void queryModeDoesNotWarnAtRegistration() { + // Query mode can't be checked upfront — items arrive in + // batches. A registration-time warning would be wrong (we + // don't know the full set) and a per-batch warning would + // flood. Stay silent. + TestLoggerFactory.getTestLogger(FormAIController.class).clearAll(); + var apollo = new FormTestFields.Project("P-1", "Apollo"); + var dup = new FormTestFields.Project("P-2", "Apollo"); + var combo = new SingleSelectField(); + combo.setItemLabelGenerator(FormTestFields.Project::name); + var controller = new FormAIController(new Div(combo)); + + controller.fieldValueOptions(ValueOptions.forField(combo) + .options((filter, limit) -> List.of(apollo, dup))); + + var warnings = TestLoggerFactory + .getTestLogger(FormAIController.class).getLoggingEvents() + .stream().filter(e -> e.getLevel() == Level.WARN).toList(); + Assertions.assertTrue(warnings.isEmpty(), + "Query-mode registration must not warn; got: " + warnings); + } + } @Nested diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormStateToolTest.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormStateToolTest.java index 646dd7bfe19..990f54af2ca 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormStateToolTest.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormStateToolTest.java @@ -684,10 +684,8 @@ void getFormStateSingleSelectWithValueOptionsRendersValueAsLabel() { combo.setItemLabelGenerator(p -> p.code() + " " + p.name()); combo.setValue(beta); var controller = new FormAIController(new Div(combo)); - controller.fieldValueOptions( - ValueOptions.forField(combo) - .options((filter, limit) -> List.of(alpha, beta)), - label -> null); + controller.fieldValueOptions(ValueOptions.forField(combo) + .options((filter, limit) -> List.of(alpha, beta))); var f = formStateFields(controller).get(0); @@ -892,8 +890,7 @@ void getFormStateValueOptionsExplicitLabelerOverridesFieldsLabelGenerator() { var controller = new FormAIController(new Div(combo)); controller.fieldValueOptions( ValueOptions.forField(combo).options(List.of(alpha, beta)) - .itemLabelGenerator(Project::code), - label -> alpha); + .itemLabelGenerator(Project::code)); var f = formStateFields(controller).get(0); var labels = new ArrayList(); @@ -917,8 +914,7 @@ void getFormStateValueOptionsUsesFieldsLabelGeneratorWhenNoExplicit() { combo.setItemLabelGenerator(Project::name); var controller = new FormAIController(new Div(combo)); controller.fieldValueOptions( - ValueOptions.forField(combo).options(List.of(alpha, beta)), - label -> alpha); + ValueOptions.forField(combo).options(List.of(alpha, beta))); var f = formStateFields(controller).get(0); var labels = new ArrayList(); @@ -936,8 +932,7 @@ void getFormStateValueOptionsFallsBackToToStringWithoutAnyLabelGenerator() { var field = new IntField(); var controller = new FormAIController(new Div(field)); controller.fieldValueOptions( - ValueOptions.forField(field).options(List.of(1, 2, 3)), - Integer::parseInt); + ValueOptions.forField(field).options(List.of(1, 2, 3))); var f = formStateFields(controller).get(0); var labels = new ArrayList(); @@ -958,11 +953,8 @@ void getFormStateValueOptionsExplicitLabelerAlsoDrivesValueRendering() { combo.setItemLabelGenerator(Project::name); combo.setValue(alpha); var controller = new FormAIController(new Div(combo)); - controller - .fieldValueOptions( - ValueOptions.forField(combo).options(List.of(alpha)) - .itemLabelGenerator(Project::code), - label -> alpha); + controller.fieldValueOptions(ValueOptions.forField(combo) + .options(List.of(alpha)).itemLabelGenerator(Project::code)); var f = formStateFields(controller).get(0); @@ -984,7 +976,7 @@ void getFormStateValueOptionsExplicitLabelerThrowingFallsBackToToString() { controller.fieldValueOptions(ValueOptions.forField(combo) .options(List.of(alpha)).itemLabelGenerator(p -> { throw new RuntimeException("boom"); - }), label -> alpha); + })); var f = formStateFields(controller).get(0); var labels = new ArrayList(); @@ -1004,8 +996,7 @@ void getFormStateValueOptionsExplicitLabelerReturningNullFallsBackToToString() { var combo = new SingleSelectField(); var controller = new FormAIController(new Div(combo)); controller.fieldValueOptions(ValueOptions.forField(combo) - .options(List.of(alpha)).itemLabelGenerator(p -> null), - label -> alpha); + .options(List.of(alpha)).itemLabelGenerator(p -> null)); var f = formStateFields(controller).get(0); var labels = new ArrayList(); @@ -1029,8 +1020,7 @@ void getFormStateMultiSelectValueOptionsLabelerAppliesToItemsEnumAndValue() { var controller = new FormAIController(new Div(multi)); controller.fieldValueOptions( ValueOptions.forField(multi).options(List.of(apollo, vega)) - .itemLabelGenerator(Project::code), - label -> apollo); + .itemLabelGenerator(Project::code)); var f = formStateFields(controller).get(0); var enumLabels = new ArrayList(); @@ -1194,8 +1184,7 @@ void getFormStateRendersStringValueWhenValueOptionsOverrideType() { field.setValue(2); var controller = new FormAIController(new Div(field)); controller.fieldValueOptions( - ValueOptions.forField(field).options(List.of(1, 2, 3)), - Integer::parseInt); + ValueOptions.forField(field).options(List.of(1, 2, 3))); var f = formStateFields(controller).get(0); diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormTestFields.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormTestFields.java index 110c6fd4742..b42f2b48b6b 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormTestFields.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormTestFields.java @@ -105,8 +105,7 @@ static class CompositeField extends StubField /** * Integer-valued field used to exercise {@code fieldValueOptions} with a - * non-{@link String} value type, where {@code toValue} must convert the - * chosen label into the field's actual value type. + * non-{@link String} value type. */ @Tag("int-field") static class IntField extends StubField { diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormValueConverterTest.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormValueConverterTest.java index d93e98e1294..9990619f940 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormValueConverterTest.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormValueConverterTest.java @@ -19,6 +19,8 @@ import java.time.LocalDate; import java.time.LocalDateTime; import java.time.LocalTime; +import java.util.ArrayList; +import java.util.List; import java.util.Set; import java.util.function.Function; @@ -232,9 +234,9 @@ void convert_emailRoutesThroughStringBranch() { @Test void convert_singleSelectWithoutFieldValueOptions_rejectedWithRegistrationHint() { - // Without a fieldValueOptions(...) registration, the LLM has no labels - // to pick and the converter has no toValue function to resolve them - // — fail loudly and point the developer at the right API. + // Without a fieldValueOptions(...) registration and no eager + // setItems(...), the field has no option source at all — fail loudly + // and point the developer at the right API. var field = wrap(new SingleSelectField(), FormFieldType.SINGLE_SELECT); @@ -300,25 +302,25 @@ void convert_multiSelectFromItems_nonStringArrayElementRejected() { } @Test - void convert_singleSelectWithValueOptionsToValue_resolvesLabel() { - // The LLM sends a label string; the registered toValue resolves it - // to the field's actual value type. Verifies the converter routes - // SINGLE_SELECT through valueOptionsToValue instead of doing - // type-driven parsing that would hand setValue a raw String. + void convert_singleSelectWithObservedItems_resolvesLabel() { + // The converter walks the registration's items, applies the labeler + // per item, and returns the first whose label matches. + var apollo = new Project("P-1", "Apollo"); var field = wrap(new SingleSelectField(), FormFieldType.SINGLE_SELECT, - hintsWithToValue(label -> new Project("P-1", label))); + hintsWithItems(List.of(apollo), Project::name)); var result = FormValueConverter.convert(field, json("\"Apollo\"")); - Assertions.assertEquals(new Project("P-1", "Apollo"), result); + Assertions.assertSame(apollo, result); } @Test void convert_singleSelectNonStringJsonRejected() { + var apollo = new Project("P-1", "Apollo"); var field = wrap(new SingleSelectField(), FormFieldType.SINGLE_SELECT, - hintsWithToValue(label -> new Project("P-1", label))); + hintsWithItems(List.of(apollo), Project::name)); var json = json("42"); Assertions.assertThrows(RejectedValueException.class, @@ -326,83 +328,108 @@ void convert_singleSelectNonStringJsonRejected() { } @Test - void convert_singleSelectToValueReturnsNullRejectedAsUnknownLabel() { - // Convention: returning null from toValue signals "I don't - // recognise this label". The converter must reject rather than - // pass null to setValue (which would silently clear the field). + void convert_singleSelectUnknownLabelRejected() { + // A label matching no registered item must reject rather than pass + // null to setValue (which would silently clear the field). + var apollo = new Project("P-1", "Apollo"); var field = wrap(new SingleSelectField(), - FormFieldType.SINGLE_SELECT, hintsWithToValue(label -> null)); + FormFieldType.SINGLE_SELECT, + hintsWithItems(List.of(apollo), Project::name)); - var json = json("\"NotAProject\""); var ex = Assertions.assertThrows(RejectedValueException.class, - () -> FormValueConverter.convert(field, json)); + () -> FormValueConverter.convert(field, + json("\"NotAProject\""))); Assertions.assertTrue(ex.getMessage().contains("NotAProject"), "Rejection reason must name the unmatched label; got: " + ex.getMessage()); } @Test - void convert_singleSelectToValueThrowsRejectedWithCuratedReason() { - // The application's toValue can throw arbitrary RuntimeException - // with arbitrary text; the converter must reject with a curated - // reason that does NOT echo the third-party exception text. + void convert_singleSelectEmptyObservedItemsRejectedWithQueryHint() { + // Empty observed-items list = query-mode registration that was + // never queried. The rejection must direct the LLM at + // query_field_options. var field = wrap(new SingleSelectField(), - FormFieldType.SINGLE_SELECT, hintsWithToValue(label -> { - throw new IllegalStateException( - "internal-detail-from-toValue"); - })); + FormFieldType.SINGLE_SELECT, + hintsWithItems(new ArrayList<>(), Project::name)); - var json = json("\"Apollo\""); var ex = Assertions.assertThrows(RejectedValueException.class, - () -> FormValueConverter.convert(field, json)); - Assertions.assertFalse( - ex.getMessage().contains("internal-detail-from-toValue"), - "Curated rejection must not echo the third-party " - + "exception text; got: " + ex.getMessage()); - Assertions.assertTrue(ex.getMessage().contains("Apollo"), - "Curated rejection should still name the offending label " - + "so the LLM can correlate; got: " + ex.getMessage()); + () -> FormValueConverter.convert(field, json("\"Apollo\""))); + Assertions.assertTrue(ex.getMessage().contains("query_field_options"), + "Empty-cache rejection must direct the LLM to call " + + "query_field_options first; got: " + ex.getMessage()); + } + + @Test + void convert_singleSelectFirstDuplicateLabelWins() { + // Duplicate labels resolve in registration order; pin so a switch + // to last-wins is caught. + var first = new Project("P-1", "Apollo"); + var dup = new Project("P-2", "Apollo"); + var field = wrap(new SingleSelectField(), + FormFieldType.SINGLE_SELECT, + hintsWithItems(List.of(first, dup), Project::name)); + + var result = FormValueConverter.convert(field, json("\"Apollo\"")); + + Assertions.assertSame(first, result); } @Test void convert_multiSelectBuildsSetFromLabels() { - // toValue maps one label to one Project; the converter aggregates - // them into a LinkedHashSet that matches the field's Set - // value type via MultiSelect-erasure. + // Each chosen label resolves to its registered item; the converter + // aggregates the matches into a LinkedHashSet. + var apollo = new Project("APL", "Apollo"); + var vega = new Project("VGA", "Vega"); var field = wrap(new MultiSelectField(), FormFieldType.MULTI_SELECT, - hintsWithToValue(label -> new Project(label, label))); + hintsWithItems(List.of(apollo, vega), Project::name)); var result = FormValueConverter.convert(field, json("[\"Apollo\", \"Vega\"]")); - Assertions.assertEquals(Set.of(new Project("Apollo", "Apollo"), - new Project("Vega", "Vega")), result); + Assertions.assertEquals(Set.of(apollo, vega), result); + } + + @Test + void convert_multiSelectPreservesLabelOrder() { + // Iteration order in the resolved set must reflect LLM-supplied + // label order, not item-registration order. + var apollo = new Project("APL", "Apollo"); + var vega = new Project("VGA", "Vega"); + var field = wrap(new MultiSelectField(), + FormFieldType.MULTI_SELECT, + hintsWithItems(List.of(apollo, vega), Project::name)); + + @SuppressWarnings("unchecked") + var result = (Set) FormValueConverter.convert(field, + json("[\"Vega\", \"Apollo\"]")); + + Assertions.assertEquals(List.of(vega, apollo), new ArrayList<>(result)); } @Test void convert_multiSelectDeduplicatesRepeatedLabels() { - // The converter aggregates into a LinkedHashSet so duplicate labels - // collapse to one item — matches the MultiSelect contract. + // Duplicate labels in the LLM's array collapse via the + // LinkedHashSet — matches the MultiSelect contract. + var apollo = new Project("APL", "Apollo"); + var vega = new Project("VGA", "Vega"); var field = wrap(new MultiSelectField(), FormFieldType.MULTI_SELECT, - hintsWithToValue(label -> new Project(label, label))); + hintsWithItems(List.of(apollo, vega), Project::name)); var result = FormValueConverter.convert(field, json("[\"Apollo\", \"Apollo\", \"Vega\"]")); - Assertions.assertEquals( - Set.of(new Project("Apollo", "Apollo"), - new Project("Vega", "Vega")), - result, - "Repeated labels must collapse into a single set entry"); + Assertions.assertEquals(Set.of(apollo, vega), result); } @Test void convert_multiSelectNonArrayJsonRejected() { + var apollo = new Project("APL", "Apollo"); var field = wrap(new MultiSelectField(), FormFieldType.MULTI_SELECT, - hintsWithToValue(label -> new Project("c", label))); + hintsWithItems(List.of(apollo), Project::name)); var json = json("\"Apollo\""); Assertions.assertThrows(RejectedValueException.class, @@ -414,9 +441,10 @@ void convert_multiSelectJsonNullReturnsFieldEmptyValue() { // JSON null on a multi-select clears the selection — the null // short-circuit at the top of convert() must win over the // multi-select array-shape enforcement. + var apollo = new Project("APL", "Apollo"); var field = wrap(new MultiSelectField(), FormFieldType.MULTI_SELECT, - hintsWithToValue(label -> new Project("c", label))); + hintsWithItems(List.of(apollo), Project::name)); var result = FormValueConverter.convert(field, json("null")); @@ -425,12 +453,10 @@ void convert_multiSelectJsonNullReturnsFieldEmptyValue() { @Test void convert_multiSelectEmptyArrayBuildsEmptySet() { - // Empty array is the LLM clearing the multi-select; the converter - // produces an empty LinkedHashSet which the field accepts as the - // cleared selection. + var apollo = new Project("APL", "Apollo"); var field = wrap(new MultiSelectField(), FormFieldType.MULTI_SELECT, - hintsWithToValue(label -> new Project("c", label))); + hintsWithItems(List.of(apollo), Project::name)); var result = FormValueConverter.convert(field, json("[]")); @@ -439,9 +465,10 @@ void convert_multiSelectEmptyArrayBuildsEmptySet() { @Test void convert_multiSelectArrayElementNotStringRejected() { + var apollo = new Project("APL", "Apollo"); var field = wrap(new MultiSelectField(), FormFieldType.MULTI_SELECT, - hintsWithToValue(label -> new Project("c", label))); + hintsWithItems(List.of(apollo), Project::name)); var json = json("[42]"); Assertions.assertThrows(RejectedValueException.class, @@ -450,50 +477,28 @@ void convert_multiSelectArrayElementNotStringRejected() { @Test void convert_multiSelectElementUnknownLabelRejected() { + var apollo = new Project("APL", "Apollo"); var field = wrap(new MultiSelectField(), - FormFieldType.MULTI_SELECT, hintsWithToValue(label -> null)); + FormFieldType.MULTI_SELECT, + hintsWithItems(List.of(apollo), Project::name)); - var json = json("[\"Apollo\", \"Unknown\"]"); var ex = Assertions.assertThrows(RejectedValueException.class, - () -> FormValueConverter.convert(field, json)); - Assertions.assertTrue(ex.getMessage().contains("Apollo"), - "Rejection reason must name the first unmatched label so " - + "the LLM knows which entry to fix; got: " + () -> FormValueConverter.convert(field, + json("[\"Apollo\", \"Unknown\"]"))); + Assertions.assertTrue(ex.getMessage().contains("Unknown"), + "Rejection reason must name the unmatched label so the " + + "LLM knows which entry to fix; got: " + ex.getMessage()); } @Test - void convert_multiSelectToValueThrowsRejectedWithCuratedReason() { - // The application's toValue can throw arbitrary text on any label - // inside the array. The rejection reason must not echo that text - // back to the LLM, but it should still name the offending label - // so the LLM can correlate. - var field = wrap(new MultiSelectField(), - FormFieldType.MULTI_SELECT, hintsWithToValue(label -> { - throw new IllegalStateException( - "internal-detail-from-toValue"); - })); - - var json = json("[\"Apollo\"]"); - var ex = Assertions.assertThrows(RejectedValueException.class, - () -> FormValueConverter.convert(field, json)); - Assertions.assertFalse( - ex.getMessage().contains("internal-detail-from-toValue"), - "Curated rejection must not echo the third-party " - + "exception text; got: " + ex.getMessage()); - Assertions.assertTrue(ex.getMessage().contains("Apollo"), - "Curated rejection should still name the offending label; " - + "got: " + ex.getMessage()); - } - - @Test - void convert_fieldValueOptionsOnPrimitiveTypeRoutesThroughToValue() { - // Even when a field's underlying type is not SINGLE_SELECT (e.g. - // an Integer-typed text field), registering fieldValueOptions(...) - // makes the LLM speak in labels. The converter must apply toValue - // instead of trying to parse the label as an integer. + void convert_fieldValueOptionsOnPrimitiveTypeRoutesThroughItems() { + // Even when a field's underlying type is not SINGLE_SELECT (e.g. an + // Integer-typed text field), registering fieldValueOptions(...) + // makes the LLM speak in labels. The converter must resolve via + // the items list instead of parsing the label as an integer. var field = wrap(new IntField(), FormFieldType.INTEGER, - hintsWithToValue(label -> "low".equals(label) ? 1 : 10)); + hintsWithItems(List.of(1, 10), v -> v == 1 ? "low" : "high")); Assertions.assertEquals(1, FormValueConverter.convert(field, json("\"low\""))); @@ -557,10 +562,13 @@ private static FormFieldDescriptor wrap(HasValue field, false, false); } - private static FormFieldHints hintsWithToValue( - Function toValue) { + private static FormFieldHints hintsWithItems(List items, + Function labeler) { var hints = new FormFieldHints(); - hints.valueOptionsToValue = toValue; + hints.valueOptionsItems = new ArrayList<>(items); + @SuppressWarnings({ "unchecked", "rawtypes" }) + Function typed = (Function) labeler; + hints.itemLabelGenerator = typed; return hints; } diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/QueryFieldOptionsToolTest.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/QueryFieldOptionsToolTest.java index c7c026c481b..6aae52d9312 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/QueryFieldOptionsToolTest.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/QueryFieldOptionsToolTest.java @@ -334,13 +334,9 @@ void queryFieldOptionsRendersItemsViaExplicitItemLabelGenerator() { var combo = new SingleSelectField(); combo.setItemLabelGenerator(Project::name); var controller = new FormAIController(new Div(combo)); - controller - .fieldValueOptions( - ValueOptions.forField(combo) - .options( - (filter, limit) -> List.of(alpha, beta)) - .itemLabelGenerator(Project::code), - label -> alpha); + controller.fieldValueOptions(ValueOptions.forField(combo) + .options((filter, limit) -> List.of(alpha, beta)) + .itemLabelGenerator(Project::code)); controller.onRequest(); var result = executeQueryFieldOptions(controller, combo, "", 10); @@ -360,7 +356,7 @@ void queryFieldOptionsRendersItemsViaFieldsItemLabelGeneratorByDefault() { combo.setItemLabelGenerator(Project::name); var controller = new FormAIController(new Div(combo)); controller.fieldValueOptions(ValueOptions.forField(combo) - .options((filter, limit) -> List.of(alpha)), label -> alpha); + .options((filter, limit) -> List.of(alpha))); controller.onRequest(); var result = executeQueryFieldOptions(controller, combo, "", 10); From 83752f6c847a95716ca8512ad946a5a5d96f0129 Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Fri, 26 Jun 2026 15:50:24 +0300 Subject: [PATCH 4/4] refactor: use map to hold entries --- .../component/ai/form/FormAIController.java | 113 +++++++++---- .../component/ai/form/FormFieldHints.java | 42 +++-- .../component/ai/form/FormValueConverter.java | 13 +- .../component/ai/form/FillFormToolTest.java | 153 ++++++++++++++++++ .../ai/form/FormValueConverterTest.java | 7 +- 5 files changed, 276 insertions(+), 52 deletions(-) diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormAIController.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormAIController.java index fc38d561be5..7347a117451 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormAIController.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormAIController.java @@ -391,7 +391,8 @@ private FormAIController applyValueOptions(ValueOptions config) { // forField(HasValue) accepts MultiSelect fields whose static reference // is upcast. Reject so the typed MultiSelect overload is the only // entry for multi-select registrations. - var isMultiSelect = config.field() instanceof MultiSelect; + var field = config.field(); + var isMultiSelect = field instanceof MultiSelect; if (!config.isMulti() && isMultiSelect) { throw new IllegalArgumentException( "Field implements MultiSelect — declare the reference as " @@ -400,49 +401,89 @@ private FormAIController applyValueOptions(ValueOptions config) { } // Collection-valued fields must implement MultiSelect — otherwise // there is no defined aggregation for the resolved items. - if (!isMultiSelect - && config.field().getEmptyValue() instanceof Collection) { + if (!isMultiSelect && field.getEmptyValue() instanceof Collection) { throw new IllegalArgumentException( "Field's value type is a Collection but the field does " + "not implement MultiSelect. Collection-valued " + "fields must implement MultiSelect to be " + "registered via fieldValueOptions(...)."); } - var labeler = resolveItemLabeler(config.field(), - config.itemLabelGenerator()); - var hints = hintsFor(config.field()); - hints.itemLabelGenerator = labeler; - // Items the converter resolves chosen labels against. Pre-populated - // with the fixed list, or appended on each query-callback invocation. - var observedItems = new ArrayList<>(); - hints.valueOptionsItems = observedItems; + var hints = hintsFor(field); + var explicit = config.itemLabelGenerator(); if (fixed != null) { @SuppressWarnings({ "unchecked", "rawtypes" }) List items = (List) fixed; - observedItems.addAll(items); - warnOnDuplicateLabels(items, labeler); - hints.valueOptionsQuery = (filter, limit) -> filterAndLimit(items, - filter, limit, labeler); hints.fixedOptions = true; + hints.valueOptionsTurnSetup = () -> rebindFixedOptions(hints, field, + explicit, items); + hints.valueOptionsTurnSetup.run(); + warnOnDuplicateLabels(items, hints.itemLabelGenerator); } else { @SuppressWarnings({ "unchecked", "rawtypes" }) BiFunction> rawQuery = (BiFunction) query; - hints.valueOptionsQuery = (filter, limit) -> { - var batch = rawQuery.apply(filter, limit); - observedItems.addAll(batch); - return batch.stream().map(labeler).toList(); - }; hints.fixedOptions = false; + hints.valueOptionsTurnSetup = () -> rebindQueryOptions(hints, field, + explicit, rawQuery); + hints.valueOptionsTurnSetup.run(); } return this; } + /** + * Rebuilds the fixed-options bindings on {@code hints} from the original + * item list and the current state of {@code field} / {@code explicit}. + * Invoked at registration and again on every {@link #onRequest()} so the + * labeler captured in {@link FormFieldHints#itemLabelGenerator} reflects + * the field's current {@code setItemLabelGenerator(...)}. + */ + private static void rebindFixedOptions(FormFieldHints hints, + HasValue field, ItemLabelGenerator explicit, + List items) { + var labeler = resolveItemLabeler(field, explicit); + hints.itemLabelGenerator = labeler; + var map = new LinkedHashMap(items.size()); + for (var item : items) { + map.putIfAbsent(labeler.apply(item), item); + } + hints.valueOptionsItems = map; + hints.valueOptionsQuery = (filter, limit) -> filterLabels(map.keySet(), + filter, limit); + } + + /** + * Rebuilds the query-options bindings on {@code hints} and resets the + * observed-items map. The wrapped query callback dedupes by label as + * batches arrive (first item per label wins) so repeated overlapping + * queries within the same turn don't inflate the map. + */ + private static void rebindQueryOptions(FormFieldHints hints, + HasValue field, ItemLabelGenerator explicit, + BiFunction> rawQuery) { + var labeler = resolveItemLabeler(field, explicit); + hints.itemLabelGenerator = labeler; + var map = new LinkedHashMap(); + hints.valueOptionsItems = map; + hints.valueOptionsQuery = (filter, limit) -> { + var batch = rawQuery.apply(filter, limit); + var labels = new ArrayList(batch.size()); + for (var item : batch) { + var label = labeler.apply(item); + labels.add(label); + map.putIfAbsent(label, item); + } + return labels; + }; + } + /** * Logs a warning when two or more items in a fixed-options registration - * render to the same label. Resolution falls back to first-in-list - * ordering, so duplicates are recoverable but ambiguous — a unique + * render to the same label. Resolution under + * {@link FormFieldHints#valueOptionsItems} keeps only the first-per-label + * (the Map's {@code putIfAbsent} semantic), so duplicates are recoverable + * but ambiguous — a unique * {@link ValueOptions#itemLabelGenerator(ItemLabelGenerator)} is the - * unambiguous fix. + * unambiguous fix. Fires once at registration; a labeler swap between turns + * that introduces new duplicates does not re-warn. */ private static void warnOnDuplicateLabels(List items, Function labeler) { @@ -469,7 +510,8 @@ private static void warnOnDuplicateLabels(List items, * Priority: an explicit {@link ItemLabelGenerator} on the registration, * otherwise the field's own {@code getItemLabelGenerator()} (via * {@link FormValueConverter#renderItem}, which also covers the - * {@link String#valueOf} fallback). + * {@link String#valueOf} fallback). Called once per turn at + * {@link #onRequest()} so a swap on the field between turns is picked up. */ private static Function resolveItemLabeler( HasValue field, ItemLabelGenerator explicit) { @@ -729,10 +771,23 @@ public void onRequest() { // are picked up. attachIds(); seedDescriptionsFromBinder(); + refreshValueOptionsBindings(); lockFields(); snapshotPreTurnValues(); } + /** + * Re-runs each value-options registration's setup at the start of a turn. + * Captures the current item-label generator from the field so a swap + * between turns is reflected, repopulates the fixed-options map from its + * immutable list, and clears the query-options map so each turn starts with + * a fresh cache. + */ + private void refreshValueOptionsBindings() { + hintsById.values().stream().map(hints -> hints.valueOptionsTurnSetup) + .filter(Objects::nonNull).forEach(Runnable::run); + } + @Override public void onResponse(Throwable error) { try { @@ -978,13 +1033,13 @@ private static String getOrCreateId(HasValue field) { return id; } - private static List filterAndLimit(List source, - String filter, int limit, Function labeler) { - var stream = source.stream().map(labeler); + private static List filterLabels(Collection labels, + String filter, int limit) { + var stream = labels.stream(); if (filter != null && !filter.isEmpty()) { var needle = filter.toLowerCase(Locale.ROOT); - stream = stream - .filter(o -> o.toLowerCase(Locale.ROOT).contains(needle)); + stream = stream.filter( + label -> label.toLowerCase(Locale.ROOT).contains(needle)); } return stream.limit(limit).toList(); } diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldHints.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldHints.java index df0a06c7f90..cc88f9d81f9 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldHints.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormFieldHints.java @@ -16,6 +16,7 @@ package com.vaadin.flow.component.ai.form; import java.util.List; +import java.util.Map; import java.util.function.BiFunction; import java.util.function.Function; @@ -40,25 +41,25 @@ final class FormFieldHints { */ BiFunction> valueOptionsQuery; /** - * Items the controller has seen for this registration: the fixed list for - * {@link ValueOptions#options(java.util.Collection)}, or items accumulated - * from {@link ValueOptions#options(java.util.function.BiFunction)} batches. - * {@link FormValueConverter} walks this list at fill time, applies - * {@link #itemLabelGenerator} per item, and returns the first whose label - * matches the LLM-supplied one (insertion order — first-wins on - * duplicates). Non-{@code null} whenever {@link #valueOptionsQuery} is set; - * empty until the query callback runs for query-mode registrations. + * Items the controller has seen for this registration, keyed by their + * LLM-facing label. {@link FormValueConverter} resolves a chosen label at + * fill time via {@link Map#get(Object)} — O(1) — and the first put per + * label wins. Populated upfront for fixed-options registrations and as each + * query-callback batch arrives for query-mode registrations. Iteration + * order is insertion order. Reset at each + * {@link FormAIController#onRequest()} turn boundary; non-{@code null} + * whenever {@link #valueOptionsQuery} is set. */ - List valueOptionsItems; + Map valueOptionsItems; /** * Item-to-label function used to render the field's current value and to - * resolve LLM-supplied labels back to items via {@link #valueOptionsItems}. - * Resolved at registration to the explicit + * compute the keys in {@link #valueOptionsItems}. Captured at each + * {@link FormAIController#onRequest()} from the explicit * {@link ValueOptions#itemLabelGenerator(com.vaadin.flow.component.ItemLabelGenerator)} - * or to a delegate that defers to {@link FormValueConverter#renderItem} - * (field's own {@code getItemLabelGenerator()}, then - * {@link String#valueOf(Object)}). Non-{@code null} whenever - * {@link #valueOptionsQuery} is set. + * if set, otherwise from the field's own {@code getItemLabelGenerator()} + * (read reflectively), otherwise {@link String#valueOf(Object)}. Stable + * within a turn so {@link #valueOptionsItems}' keys remain valid for + * lookup. Non-{@code null} whenever {@link #valueOptionsQuery} is set. */ Function itemLabelGenerator; /** @@ -67,5 +68,16 @@ final class FormFieldHints { * vs {@code queryable} choice in {@link FormFieldSchema}. */ boolean fixedOptions; + /** + * Rebuilds {@link #valueOptionsItems}, {@link #valueOptionsQuery}, and + * {@link #itemLabelGenerator} from the registration's captured config + * (fixed list or query callback, explicit labeler or field reference). + * Invoked once at registration so the schema works before the first turn, + * and again at each {@link FormAIController#onRequest()} so a labeler + * change on the field between turns is picked up and the query-mode map + * starts each turn empty. {@code null} when no value-options registration + * exists for this field. + */ + Runnable valueOptionsTurnSetup; boolean ignored; } diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormValueConverter.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormValueConverter.java index 2fb61aa6e5d..a047159a31d 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormValueConverter.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormValueConverter.java @@ -300,18 +300,17 @@ private static Object convertMultiSelect(FormFieldDescriptor field, } /** - * Walks {@link FormFieldHints#valueOptionsItems} and returns the first item - * whose label (via {@link FormFieldHints#itemLabelGenerator}) matches the - * LLM-supplied one. An empty list is called out with a hint at + * Returns the item that was indexed under the LLM-supplied label in + * {@link FormFieldHints#valueOptionsItems}. An empty map is the query-mode + * "not queried yet this turn" case and is called out with a hint at * {@code query_field_options}, since that's the only way the LLM could have * reached this point without seeing options. */ private static Object resolveLabelAgainstObservedItems(String label, FormFieldHints hints) { - for (var item : hints.valueOptionsItems) { - if (label.equals(hints.itemLabelGenerator.apply(item))) { - return item; - } + var item = hints.valueOptionsItems.get(label); + if (item != null) { + return item; } if (hints.valueOptionsItems.isEmpty()) { throw new RejectedValueException("No matching option for label: " diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FillFormToolTest.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FillFormToolTest.java index 5cab42cabc1..c4a56893f4c 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FillFormToolTest.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FillFormToolTest.java @@ -1603,6 +1603,159 @@ void fillForm_multiSelect_reregistrationOverwritesPriorOptions() { + "registration's items, not the first"); } + @Test + void fillForm_singleSelect_duplicateLabelsResolveToFirstItem() { + // When two items render to the same label, the converter resolves + // to the first item in registration order — pins the contract the + // duplicate-label warning advertises. + var first = new Project("P-1", "Apollo"); + var dup = new Project("P-2", "Apollo"); + var field = new SingleSelectField(); + field.setItemLabelGenerator(Project::name); + var controller = newController(field); + controller.fieldValueOptions( + ValueOptions.forField(field).options(List.of(first, dup))); + controller.onRequest(); + + var result = fillFormResult(controller, payload(field, "\"Apollo\"")); + + Assertions.assertSame(first, field.getValue(), + "First item per label must win on resolution"); + Assertions.assertTrue(success(result)); + } + + @Test + void fillForm_singleSelect_labelerChangeBetweenTurnsIsReflected() { + // The controller captures the field's labeler at each onRequest, so + // a swap between turns is reflected in both the labels the LLM is + // offered and the inverse lookup at fill time. + var apollo = new Project("APL", "Apollo"); + var field = new SingleSelectField(); + field.setItemLabelGenerator(Project::name); + var controller = newController(field); + controller.fieldValueOptions( + ValueOptions.forField(field).options(List.of(apollo))); + controller.onRequest(); + fillFormResult(controller, payload(field, "\"Apollo\"")); + Assertions.assertEquals(apollo, field.getValue(), + "Turn 1 must resolve via Project::name"); + + field.setValue(null); + field.setItemLabelGenerator(Project::code); + controller.onRequest(); + var result = fillFormResult(controller, payload(field, "\"APL\"")); + + Assertions.assertEquals(apollo, field.getValue(), + "Turn 2 must resolve via the newly-installed Project::code"); + Assertions.assertTrue(success(result)); + } + + @Test + void fillForm_singleSelect_queryModeFillBeforeQueryIsRejected() { + // A query-mode field that has not been queried yet has an empty + // cache. The fill must reject with a reason that names + // query_field_options so the LLM knows the missing step. + var apollo = new Project("APL", "Apollo"); + var field = new SingleSelectField(); + field.setItemLabelGenerator(Project::name); + var controller = newController(field); + controller.fieldValueOptions(ValueOptions.forField(field) + .options((filter, limit) -> List.of(apollo))); + controller.onRequest(); + + var result = fillFormResult(controller, payload(field, "\"Apollo\"")); + + Assertions.assertNull(field.getValue()); + var reason = rejectionReason(result, idOf(field)); + Assertions.assertTrue(reason.contains("query_field_options"), + "Empty-cache rejection must direct the LLM at " + + "query_field_options; got: " + reason); + } + + @Test + void fillForm_singleSelect_queryModeCachePersistsWithinTurn() { + // Within one turn the cache accumulates across query batches so + // the LLM can fill against a label seen in an earlier batch of the + // same turn. + var apollo = new Project("APL", "Apollo"); + var vega = new Project("VGA", "Vega"); + var catalog = List.of(apollo, vega); + var field = new SingleSelectField(); + field.setItemLabelGenerator(Project::name); + var controller = newController(field); + controller.fieldValueOptions(ValueOptions.forField(field) + .options((filter, limit) -> catalog.stream() + .filter(p -> p.name().toLowerCase() + .contains(filter.toLowerCase())) + .limit(limit).toList())); + controller.onRequest(); + executeQueryFieldOptions(controller, field, "Apo", 10); + executeQueryFieldOptions(controller, field, "Veg", 10); + + var result = fillFormResult(controller, payload(field, "\"Apollo\"")); + + Assertions.assertEquals(apollo, field.getValue(), + "Items from earlier queries in the same turn must remain " + + "resolvable"); + Assertions.assertTrue(success(result)); + } + + @Test + void fillForm_singleSelect_queryModeCacheResetsBetweenTurns() { + // The cache is bound to the turn: a fresh onRequest clears it so a + // second turn that skips query_field_options is rejected even if + // the first turn populated the cache for the same label. + var apollo = new Project("APL", "Apollo"); + var field = new SingleSelectField(); + field.setItemLabelGenerator(Project::name); + var controller = newController(field); + controller.fieldValueOptions(ValueOptions.forField(field) + .options((filter, limit) -> List.of(apollo))); + controller.onRequest(); + executeQueryFieldOptions(controller, field, "", 10); + fillFormResult(controller, payload(field, "\"Apollo\"")); + Assertions.assertEquals(apollo, field.getValue(), + "Turn 1 must fill after the query populates the cache"); + + field.setValue(null); + controller.onRequest(); + var result = fillFormResult(controller, payload(field, "\"Apollo\"")); + + Assertions.assertNull(field.getValue(), + "Turn 2 must reject because the cache reset at onRequest"); + var reason = rejectionReason(result, idOf(field)); + Assertions.assertTrue(reason.contains("query_field_options"), + "Reason must direct the LLM at query_field_options; got: " + + reason); + } + + @Test + void fillForm_singleSelect_queryModeRepeatedOverlappingQueriesDoNotDuplicate() { + // Repeated queries returning overlapping items must not corrupt + // resolution. With first-per-label dedup in the cache, the LLM + // resolves to the original item regardless of how many times the + // application's BiFunction returned it. + var first = new Project("APL", "Apollo"); + var second = new Project("APL-NEW", "Apollo"); + var versions = new java.util.concurrent.atomic.AtomicReference<>(first); + var field = new SingleSelectField(); + field.setItemLabelGenerator(Project::name); + var controller = newController(field); + controller.fieldValueOptions(ValueOptions.forField(field) + .options((filter, limit) -> List.of(versions.get()))); + controller.onRequest(); + executeQueryFieldOptions(controller, field, "", 10); + versions.set(second); + executeQueryFieldOptions(controller, field, "", 10); + + var result = fillFormResult(controller, payload(field, "\"Apollo\"")); + + Assertions.assertSame(first, field.getValue(), + "Overlapping queries within a turn keep the first item " + + "per label; the later instance must not replace it"); + Assertions.assertTrue(success(result)); + } + @Test void fillForm_fieldValueOptionsOnPrimitiveTypeSkipsTypeDrivenParsing() { // Registering fieldValueOptions(...) on a non-SELECT field still makes diff --git a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormValueConverterTest.java b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormValueConverterTest.java index 9990619f940..4092287115b 100644 --- a/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormValueConverterTest.java +++ b/vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormValueConverterTest.java @@ -20,6 +20,7 @@ import java.time.LocalDateTime; import java.time.LocalTime; import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; import java.util.Set; import java.util.function.Function; @@ -565,7 +566,11 @@ private static FormFieldDescriptor wrap(HasValue field, private static FormFieldHints hintsWithItems(List items, Function labeler) { var hints = new FormFieldHints(); - hints.valueOptionsItems = new ArrayList<>(items); + var map = new LinkedHashMap(); + for (var item : items) { + map.putIfAbsent(labeler.apply(item), item); + } + hints.valueOptionsItems = map; @SuppressWarnings({ "unchecked", "rawtypes" }) Function typed = (Function) labeler; hints.itemLabelGenerator = typed;