Skip to content

Commit 435996d

Browse files
tomivirkkiclaude
andauthored
fix: resolve eager-items selects without explicit valueOptions (#9357)
## Summary - An eager `setItems(...)` ComboBox/Select/RadioButtonGroup/CheckboxGroup/MultiSelectComboBox was unwritable from `fill_form` unless the app also called `FormAIController.valueOptions(...)`, even though the schema already surfaces those items as `enum`. - The converter now resolves an LLM-supplied label against the field's in-memory items via `renderItem` for both single- and multi-select, so both halves of the tool protocol agree on the label set without forcing a second registration. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 983b25e commit 435996d

3 files changed

Lines changed: 255 additions & 16 deletions

File tree

vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/main/java/com/vaadin/flow/component/ai/form/FormValueConverter.java

Lines changed: 88 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -191,9 +191,12 @@ static final class RejectedValueException extends RuntimeException {
191191
* the resulting object matches the field's value type before
192192
* {@link HasValue#setValue} sees it. Multi-select fields expect a JSON
193193
* array of labels; single-select and other label-routed fields expect a
194-
* single string. Selection fields without a {@code valueOptions}
195-
* registration are rejected with a curated reason naming the missing
196-
* registration.
194+
* single string. A selection field without a {@code valueOptions}
195+
* registration falls back to matching the supplied label(s) against the
196+
* field's own in-memory items (an eager {@code setItems(...)}), which carry
197+
* the same labels {@code get_form_state} advertised; only a field that has
198+
* neither a {@code valueOptions} registration nor items is rejected with a
199+
* curated reason naming the missing registration.
197200
*/
198201
static Object convert(FormFieldDescriptor field, JsonNode value) {
199202
if (value == null || value.isNull()) {
@@ -221,15 +224,41 @@ static Object convert(FormFieldDescriptor field, JsonNode value) {
221224
case DATE -> convertDate(value);
222225
case DATE_TIME -> convertDateTime(value);
223226
case TIME -> convertTime(value);
224-
case SINGLE_SELECT -> throw new RejectedValueException(
225-
"Selection field has no value options registered — register "
226-
+ "options via FormAIController.valueOptions(...) "
227-
+ "so the AI knows what to pick.");
227+
case SINGLE_SELECT -> convertSingleSelectFromItems(field, value);
228228
default -> throw new RejectedValueException(
229229
"Unsupported field type: " + field.type());
230230
};
231231
}
232232

233+
/**
234+
* Fallback resolver for a SINGLE_SELECT field that has no
235+
* {@code valueOptions(...)} registered: matches the LLM-supplied label
236+
* against the field's in-memory items via {@link #renderItem}. This is the
237+
* inverse of the schema path that emits the same items as an {@code enum}
238+
* array — both sides agree on the label set, so the LLM can write through
239+
* an eager {@code setItems(...)} field without the developer wiring up
240+
* {@code valueOptions(...)} too.
241+
*/
242+
private static Object convertSingleSelectFromItems(
243+
FormFieldDescriptor field, JsonNode value) {
244+
if (!value.isString()) {
245+
throw new RejectedValueException(
246+
"Expected string label, got " + value);
247+
}
248+
var items = listDataProviderItems(field.field());
249+
if (items.isEmpty()) {
250+
// No items and no valueOptions — the field has no option source at
251+
// all. Point the developer at the missing wiring rather than the
252+
// missing match, since the model couldn't have picked any label.
253+
throw new RejectedValueException(
254+
"Selection field has no value options registered — register "
255+
+ "options via FormAIController.valueOptions(...) "
256+
+ "or call setItems(...) so the AI knows what to "
257+
+ "pick.");
258+
}
259+
return resolveLabelAgainstItems(field.field(), value.asString(), items);
260+
}
261+
233262
private static Object convertSingleLabel(JsonNode value,
234263
Function<String, ?> toValue) {
235264
if (!value.isString()) {
@@ -241,15 +270,6 @@ private static Object convertSingleLabel(JsonNode value,
241270

242271
private static Object convertMultiSelect(FormFieldDescriptor field,
243272
JsonNode value) {
244-
var hints = field.hints();
245-
var toValue = hints != null ? hints.valueOptionsToValue : null;
246-
if (toValue == null) {
247-
throw new RejectedValueException(
248-
"Multi-select field has no value options registered — "
249-
+ "register options via "
250-
+ "FormAIController.valueOptions(...) so the AI "
251-
+ "knows what to pick.");
252-
}
253273
if (!value.isArray()) {
254274
throw new RejectedValueException(
255275
"Expected array of string labels, got " + value);
@@ -260,6 +280,11 @@ private static Object convertMultiSelect(FormFieldDescriptor field,
260280
if (value.isEmpty()) {
261281
return field.field().getEmptyValue();
262282
}
283+
var hints = field.hints();
284+
var toValue = hints != null ? hints.valueOptionsToValue : null;
285+
if (toValue == null) {
286+
return convertMultiSelectFromItems(field, value);
287+
}
263288
var result = new LinkedHashSet<>();
264289
for (var node : value) {
265290
if (!node.isString()) {
@@ -285,6 +310,53 @@ private static Object convertMultiSelect(FormFieldDescriptor field,
285310
return result;
286311
}
287312

313+
/**
314+
* Fallback resolver for a MULTI_SELECT field that has no
315+
* {@code valueOptions(...)} registered: matches each LLM-supplied label
316+
* against the field's in-memory items via {@link #renderItem}. Mirrors
317+
* {@link #convertSingleSelectFromItems} so eager-items
318+
* {@code MultiSelectComboBox<T>} and {@code CheckboxGroup<T>} are writable
319+
* without the developer wiring up {@code valueOptions(...)} too.
320+
*/
321+
private static Object convertMultiSelectFromItems(FormFieldDescriptor field,
322+
JsonNode value) {
323+
var items = listDataProviderItems(field.field());
324+
if (items.isEmpty()) {
325+
throw new RejectedValueException(
326+
"Multi-select field has no value options registered — "
327+
+ "register options via "
328+
+ "FormAIController.valueOptions(...) or call "
329+
+ "setItems(...) so the AI knows what to pick.");
330+
}
331+
var result = new LinkedHashSet<>();
332+
for (var node : value) {
333+
if (!node.isString()) {
334+
throw new RejectedValueException(
335+
"Expected string label, got " + node);
336+
}
337+
result.add(resolveLabelAgainstItems(field.field(), node.asString(),
338+
items));
339+
}
340+
return result;
341+
}
342+
343+
/**
344+
* Walks the field's in-memory items list and returns the first item whose
345+
* {@link #renderItem} matches the LLM-supplied label. The matching mirrors
346+
* what the schema sent the LLM under {@code enum}, so both halves of the
347+
* tool protocol agree on the label set.
348+
*/
349+
private static Object resolveLabelAgainstItems(HasValue<?, ?> field,
350+
String label, List<Object> items) {
351+
for (var item : items) {
352+
if (label.equals(renderItem(field, item))) {
353+
return item;
354+
}
355+
}
356+
throw new RejectedValueException(
357+
"No matching option for label: " + label);
358+
}
359+
288360
/**
289361
* Resolves one LLM-supplied label via the application's
290362
* {@code valueOptionsToValue}. Wraps the application's throw and the

vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FillFormToolTest.java

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,68 @@ void fillForm_singleSelect_withoutValueOptionsIsRejectedWithHint() {
947947
+ rejectionReason(result, idOf(field)));
948948
}
949949

950+
@Test
951+
void fillForm_singleSelect_eagerItemsResolveLabelWithoutValueOptions() {
952+
// A `ComboBox<String>`/`Select<String>` with eager
953+
// `setItems(...)` has a non-empty ListDataProvider and the schema
954+
// already surfaces its items as `enum`. The converter must resolve
955+
// an LLM-supplied label against those items via
956+
// FormValueConverter.renderItem so the field is writable without
957+
// also requiring FormAIController.valueOptions(...).
958+
var field = new SingleSelectField<String>();
959+
field.setItems("EUR", "USD", "GBP");
960+
var controller = controllerFor(field);
961+
962+
var result = fillFormResult(controller, payload(field, "\"EUR\""));
963+
964+
Assertions.assertEquals("EUR", field.getValue(),
965+
"Eager-items SINGLE_SELECT must accept a label that "
966+
+ "matches one of its items");
967+
Assertions.assertTrue(success(result));
968+
}
969+
970+
@Test
971+
void fillForm_singleSelect_eagerItemsRejectLabelNotInItems() {
972+
// Symmetric to the above: when eager items exist, a label that is
973+
// not in the rendered set must be rejected with a reason that
974+
// names the offending label so the LLM can self-correct.
975+
var field = new SingleSelectField<String>();
976+
field.setItems("EUR", "USD", "GBP");
977+
var controller = controllerFor(field);
978+
979+
var result = fillFormResult(controller, payload(field, "\"YEN\""));
980+
981+
Assertions.assertNull(field.getValue(),
982+
"Unknown label must not write a value");
983+
Assertions.assertEquals(List.of(idOf(field)), rejectedIds(result));
984+
Assertions.assertTrue(
985+
rejectionReason(result, idOf(field)).contains("YEN"),
986+
"Reason must name the unmatched label; got: "
987+
+ rejectionReason(result, idOf(field)));
988+
}
989+
990+
@Test
991+
void fillForm_singleSelect_eagerItemsResolveLabelViaItemLabelGenerator() {
992+
// The symmetry between schema (emits `enum`) and converter
993+
// (matches by label) rides on FormValueConverter.renderItem.
994+
// String items collapse to Object#toString, so an item type with
995+
// a custom ItemLabelGenerator is what actually exercises the
996+
// generator path on both halves of the protocol.
997+
var field = new SingleSelectField<Project>();
998+
var apollo = new Project("APL", "Apollo");
999+
var vega = new Project("VGA", "Vega");
1000+
field.setItems(apollo, vega);
1001+
field.setItemLabelGenerator(Project::name);
1002+
var controller = controllerFor(field);
1003+
1004+
var result = fillFormResult(controller, payload(field, "\"Apollo\""));
1005+
1006+
Assertions.assertEquals(apollo, field.getValue(),
1007+
"Eager-items SINGLE_SELECT must match via the custom "
1008+
+ "ItemLabelGenerator, not via toString()");
1009+
Assertions.assertTrue(success(result));
1010+
}
1011+
9501012
@Test
9511013
void fillForm_singleSelect_unknownLabelIsRejected() {
9521014
// toValue returning null is the agreed signal for "label doesn't
@@ -1027,6 +1089,71 @@ void fillForm_multiSelect_withoutValueOptionsIsRejectedWithHint() {
10271089
rejectionReason(result, idOf(field)).contains("valueOptions"));
10281090
}
10291091

1092+
@Test
1093+
void fillForm_multiSelect_eagerItemsResolveLabelsWithoutValueOptions() {
1094+
// Symmetric to the SINGLE_SELECT regression guard: a
1095+
// `MultiSelectComboBox<String>`/`CheckboxGroup<String>` with eager
1096+
// `setItems(...)` has a non-empty ListDataProvider and the schema
1097+
// surfaces the items as the `items.enum` of the array. The
1098+
// converter must resolve each label against those items so the
1099+
// field is writable without also requiring valueOptions(...).
1100+
var field = new MultiSelectField<String>();
1101+
field.setItems("AI", "Cloud", "Security");
1102+
var controller = controllerFor(field);
1103+
1104+
var result = fillFormResult(controller,
1105+
payload(field, "[\"AI\", \"Cloud\"]"));
1106+
1107+
Assertions.assertEquals(Set.of("AI", "Cloud"), field.getValue(),
1108+
"Eager-items MULTI_SELECT must accept labels that "
1109+
+ "match its items");
1110+
Assertions.assertTrue(success(result));
1111+
}
1112+
1113+
@Test
1114+
void fillForm_multiSelect_eagerItemsRejectAnyLabelNotInItems() {
1115+
// Unknown labels in the multi-select array must surface in the
1116+
// rejection so the LLM can drop or re-pick them; the field must
1117+
// not be partially written either.
1118+
var field = new MultiSelectField<String>();
1119+
field.setItems("AI", "Cloud", "Security");
1120+
var controller = controllerFor(field);
1121+
1122+
var result = fillFormResult(controller,
1123+
payload(field, "[\"AI\", \"Quantum\"]"));
1124+
1125+
Assertions.assertEquals(Set.of(), field.getValue(),
1126+
"A label miss must abort the multi-select write, not "
1127+
+ "leave the field with the partial set");
1128+
Assertions.assertEquals(List.of(idOf(field)), rejectedIds(result));
1129+
Assertions.assertTrue(
1130+
rejectionReason(result, idOf(field)).contains("Quantum"),
1131+
"Reason must name the unmatched label; got: "
1132+
+ rejectionReason(result, idOf(field)));
1133+
}
1134+
1135+
@Test
1136+
void fillForm_multiSelect_eagerItemsResolveLabelsViaItemLabelGenerator() {
1137+
// Same lock as the SINGLE_SELECT variant: a typed item with a
1138+
// custom ItemLabelGenerator exercises renderItem on both the
1139+
// schema-emit side and the converter-resolve side, so the test
1140+
// catches drift if either side stops honoring the generator.
1141+
var field = new MultiSelectField<Project>();
1142+
var apollo = new Project("APL", "Apollo");
1143+
var vega = new Project("VGA", "Vega");
1144+
field.setItems(apollo, vega);
1145+
field.setItemLabelGenerator(Project::name);
1146+
var controller = controllerFor(field);
1147+
1148+
var result = fillFormResult(controller,
1149+
payload(field, "[\"Apollo\", \"Vega\"]"));
1150+
1151+
Assertions.assertEquals(Set.of(apollo, vega), field.getValue(),
1152+
"Eager-items MULTI_SELECT must match via the custom "
1153+
+ "ItemLabelGenerator, not via toString()");
1154+
Assertions.assertTrue(success(result));
1155+
}
1156+
10301157
@Test
10311158
void fillForm_multiSelect_emptyArrayClearsField() {
10321159
// Empty array is the LLM clearing the multi-select; the resulting

vaadin-ai-components-flow-parent/vaadin-ai-components-flow/src/test/java/com/vaadin/flow/component/ai/form/FormValueConverterTest.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,46 @@ void convert_multiSelectWithoutValueOptions_rejectedWithRegistrationHint() {
258258
+ "registration; got: " + ex.getMessage());
259259
}
260260

261+
@Test
262+
void convert_singleSelectFromItems_nonStringJsonRejected() {
263+
// Eager-items SINGLE_SELECT (no valueOptions registered, items
264+
// populated via setItems) must reject a non-string JSON shape
265+
// before reaching renderItem-based matching — otherwise a JSON
266+
// boolean true coincidentally renders to "true" and could match
267+
// an item whose label happens to be "true", silently writing a
268+
// wrong-shape value.
269+
var field = new SingleSelectField<String>();
270+
field.setItems("true", "false");
271+
272+
var json = json("true");
273+
var ex = Assertions
274+
.assertThrows(RejectedValueException.class,
275+
() -> FormValueConverter.convert(
276+
wrap(field, FormFieldType.SINGLE_SELECT),
277+
json));
278+
Assertions.assertTrue(ex.getMessage().contains("string label"),
279+
"Rejection reason must name the type mismatch, not the "
280+
+ "label match path; got: " + ex.getMessage());
281+
}
282+
283+
@Test
284+
void convert_multiSelectFromItems_nonStringArrayElementRejected() {
285+
// Symmetric to convert_singleSelectFromItems_nonStringJsonRejected:
286+
// each element of a multi-select array must be a JSON string
287+
// before reaching renderItem-based matching against the field's
288+
// eager items.
289+
var field = new MultiSelectField<String>();
290+
field.setItems("true", "false");
291+
292+
var json = json("[true]");
293+
var ex = Assertions.assertThrows(RejectedValueException.class,
294+
() -> FormValueConverter.convert(
295+
wrap(field, FormFieldType.MULTI_SELECT), json));
296+
Assertions.assertTrue(ex.getMessage().contains("string label"),
297+
"Rejection reason must name the type mismatch, not the "
298+
+ "label match path; got: " + ex.getMessage());
299+
}
300+
261301
@Test
262302
void convert_singleSelectWithValueOptionsToValue_resolvesLabel() {
263303
// The LLM sends a label string; the registered toValue resolves it

0 commit comments

Comments
 (0)