Merge feature/declared-field-defaults into main
This commit is contained in:
commit
bf30c5b3eb
|
|
@ -40,6 +40,7 @@ import com.github.victools.jsonschema.module.jackson.JacksonSchemaModule;
|
|||
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.FieldRetriever;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.LongText;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.DefaultsWhenEmpty;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.Structural;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.UiOptionalGroup;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.DynamicSchema;
|
||||
|
|
@ -101,6 +102,7 @@ public class JsonSchemaProducer {
|
|||
Map<Class<?>, Map<String, DynamicSchema>> dynamicSchemaMap = collectDynamicSchemaMetadata(type);
|
||||
Map<Class<?>, Map<String, LongText>> longTextMap = collectLongTextMetadata(type);
|
||||
Map<Class<?>, Set<String>> acceptsPlaceholderMap = collectAcceptsVariablePlaceholderMetadata(type);
|
||||
Map<Class<?>, Set<String>> defaultsWhenEmptyMap = collectDefaultsWhenEmptyMetadata(type);
|
||||
Map<Class<?>, Map<String, Structural>> structuralMap = collectStructuralMetadata(type);
|
||||
Map<Class<?>, Map<String, UiOptionalGroup>> uiOptionalGroupMap = collectUiOptionalGroupMetadata(type);
|
||||
Map<Class<?>, Map<String, UiEnabledWhen>> uiEnabledWhenMap = collectUiEnabledWhenMetadata(type);
|
||||
|
|
@ -119,6 +121,7 @@ public class JsonSchemaProducer {
|
|||
applyDynamicSchemaMetadata(root, getMergedMetadata(dynamicSchemaMap, type));
|
||||
applyLongTextMetadata(root, getMergedMetadata(longTextMap, type));
|
||||
applyAcceptsVariablePlaceholderMetadata(root, mergedNames(acceptsPlaceholderMap, type));
|
||||
applyDefaultsWhenEmptyMetadata(root, mergedNames(defaultsWhenEmptyMap, type));
|
||||
applyStructuralMetadata(root, getMergedMetadata(structuralMap, type));
|
||||
applyUiOptionalGroupMetadata(root, getMergedMetadata(uiOptionalGroupMap, type));
|
||||
applyUiEnabledWhenMetadata(root, getMergedMetadata(uiEnabledWhenMap, type));
|
||||
|
|
@ -153,6 +156,7 @@ public class JsonSchemaProducer {
|
|||
metadataClasses.addAll(schemaAllowedValuesMap.keySet());
|
||||
metadataClasses.addAll(configurableAsInputMap.keySet());
|
||||
metadataClasses.addAll(acceptsPlaceholderMap.keySet());
|
||||
metadataClasses.addAll(defaultsWhenEmptyMap.keySet());
|
||||
for (Entry<String, JsonNode> entry : definitions.properties()) {
|
||||
if (!(entry.getValue() instanceof ObjectNode classSchema)) {
|
||||
continue;
|
||||
|
|
@ -166,6 +170,7 @@ public class JsonSchemaProducer {
|
|||
applyDynamicSchemaMetadata(classSchema, getMergedMetadata(dynamicSchemaMap, matchedClass));
|
||||
applyLongTextMetadata(classSchema, getMergedMetadata(longTextMap, matchedClass));
|
||||
applyAcceptsVariablePlaceholderMetadata(classSchema, mergedNames(acceptsPlaceholderMap, matchedClass));
|
||||
applyDefaultsWhenEmptyMetadata(classSchema, mergedNames(defaultsWhenEmptyMap, matchedClass));
|
||||
applyStructuralMetadata(classSchema, getMergedMetadata(structuralMap, matchedClass));
|
||||
applyUiOptionalGroupMetadata(classSchema, getMergedMetadata(uiOptionalGroupMap, matchedClass));
|
||||
applyUiEnabledWhenMetadata(classSchema, getMergedMetadata(uiEnabledWhenMap, matchedClass));
|
||||
|
|
@ -831,6 +836,64 @@ public class JsonSchemaProducer {
|
|||
}
|
||||
}
|
||||
|
||||
private Map<Class<?>, Set<String>> collectDefaultsWhenEmptyMetadata(Class<?> rootClass) {
|
||||
Map<Class<?>, Set<String>> result = new HashMap<>();
|
||||
Set<Class<?>> visited = new HashSet<>();
|
||||
Queue<Class<?>> queue = new ArrayDeque<>();
|
||||
queue.add(rootClass);
|
||||
|
||||
while (!queue.isEmpty()) {
|
||||
Class<?> current = queue.poll();
|
||||
if (current == null || !visited.add(current) || isTerminalType(current)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
Set<String> names = new LinkedHashSet<>();
|
||||
for (Field field : current.getDeclaredFields()) {
|
||||
if (field.getAnnotation(DefaultsWhenEmpty.class) != null) {
|
||||
names.add(field.getName());
|
||||
}
|
||||
enqueueRelatedTypes(queue, field.getGenericType(), field.getType());
|
||||
}
|
||||
|
||||
if (current.isRecord()) {
|
||||
for (RecordComponent component : current.getRecordComponents()) {
|
||||
if (component.getAnnotation(DefaultsWhenEmpty.class) != null) {
|
||||
names.add(component.getName());
|
||||
}
|
||||
enqueueRelatedTypes(queue, component.getGenericType(), component.getType());
|
||||
}
|
||||
}
|
||||
|
||||
if (current.getSuperclass() != null) {
|
||||
queue.add(current.getSuperclass());
|
||||
}
|
||||
|
||||
if (!names.isEmpty()) {
|
||||
result.put(current, names);
|
||||
}
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
private void applyDefaultsWhenEmptyMetadata(ObjectNode classSchema, Set<String> names) {
|
||||
if (names == null || names.isEmpty()) {
|
||||
return;
|
||||
}
|
||||
JsonNode propsNode = classSchema.get("properties");
|
||||
if (!(propsNode instanceof ObjectNode properties)) {
|
||||
return;
|
||||
}
|
||||
|
||||
for (String name : names) {
|
||||
JsonNode propNode = properties.get(name);
|
||||
if (propNode instanceof ObjectNode propertySchema) {
|
||||
propertySchema.put("x-ui-defaults-when-empty", true);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private Map<Class<?>, Map<String, Structural>> collectStructuralMetadata(Class<?> rootClass) {
|
||||
Map<Class<?>, Map<String, Structural>> result = new HashMap<>();
|
||||
Set<Class<?>> visited = new HashSet<>();
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ import it.cnr.isti.workflow.manager.configurations.annotations.DynamicSchema;
|
|||
import it.cnr.isti.workflow.manager.configurations.annotations.AcceptsVariablePlaceholder;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.ConfigurableAsInput;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.UiContextKeys;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.DefaultsWhenEmpty;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.SchemaAllowedValues;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.UiEnabledWhen;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.UiOptionalGroup;
|
||||
|
|
@ -160,6 +161,7 @@ public class MCPAgentBlockConfiguration extends BlockConfiguration<MCPAgentBlock
|
|||
// The default lives in effectiveSourceType() below; declared here so the editor can say
|
||||
// which one an empty field is using instead of only that it is using one.
|
||||
@SchemaAllowedValues(value = { "CATALOG", "CUSTOM" }, defaultValue = "CATALOG")
|
||||
@DefaultsWhenEmpty
|
||||
String sourceType,
|
||||
@JsonProperty(required = false)
|
||||
@UiEnabledWhen(field = "sourceType", equalsAny = { "CATALOG", "" })
|
||||
|
|
|
|||
|
|
@ -21,6 +21,7 @@ import it.cnr.isti.workflow.manager.configurations.annotations.ConfigurableAsInp
|
|||
import it.cnr.isti.workflow.manager.configurations.annotations.DynamicSchema;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.FieldRetriever;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.LongText;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.DefaultsWhenEmpty;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.SchemaAllowedValues;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.Structural;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.UiEnabledWhen;
|
||||
|
|
@ -144,6 +145,7 @@ public class MCPAgentChatBlockConfiguration extends BlockConfiguration<MCPAgentC
|
|||
// Same default as effectiveSourceType() below, declared for the editor - and the same
|
||||
// one the agent block declares, so the two dialogs read alike.
|
||||
@SchemaAllowedValues(value = { "CATALOG", "CUSTOM" }, defaultValue = "CATALOG")
|
||||
@DefaultsWhenEmpty
|
||||
String sourceType,
|
||||
@JsonProperty(required = false)
|
||||
@UiEnabledWhen(field = "sourceType", equalsAny = { "CATALOG", "" })
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import com.fasterxml.jackson.annotation.JsonIgnore;
|
|||
import com.fasterxml.jackson.annotation.JsonProperty;
|
||||
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.FieldRetriever;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.DefaultsWhenEmpty;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.SchemaAllowedValues;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.UiContextKeys;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.UiDescription;
|
||||
|
|
@ -33,6 +34,7 @@ public record MCPAgentUploadInput(
|
|||
* than only that it is using one.
|
||||
*/
|
||||
@SchemaAllowedValues(value = { SOURCE_INPUT, SOURCE_GLOBAL }, defaultValue = SOURCE_INPUT)
|
||||
@DefaultsWhenEmpty
|
||||
@UiLabel("File comes from")
|
||||
@UiDescription("Uploaded to an input of this step, or taken from a global input of the flow.")
|
||||
@JsonProperty(required = false) String source,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,31 @@
|
|||
// SPDX-FileCopyrightText: 2025-2026 Lucio Lelii <lucio.lelii@isti.cnr.it> - ISTI-CNR
|
||||
// SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
// Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM.
|
||||
|
||||
package it.cnr.isti.workflow.manager.configurations.annotations;
|
||||
|
||||
import java.lang.annotation.ElementType;
|
||||
import java.lang.annotation.Retention;
|
||||
import java.lang.annotation.RetentionPolicy;
|
||||
import java.lang.annotation.Target;
|
||||
|
||||
/**
|
||||
* Marks a field whose empty state is a decision rather than an omission, so the editor offers a way
|
||||
* back to it once the field holds a value.
|
||||
*
|
||||
* <p>Declared per field instead of inferred from the field being optional, which is what the editor
|
||||
* used to do: "you may leave this blank" and "leaving this blank means something specific" are
|
||||
* different claims, and only the second is worth a control. Most optional fields are simply blank.
|
||||
*
|
||||
* <p>The model sampling parameters are the case this exists for: empty means the provider picks, a
|
||||
* value cannot be un-typed back into that state by clearing the box - that reads as an unfinished
|
||||
* edit - and nobody knows what number to type to get the provider's own choice back.
|
||||
*
|
||||
* <p>Says nothing about what the default is. When a concrete one is known it belongs in JSON
|
||||
* Schema's own {@code default}, which the editor already shows alongside; here there is often no
|
||||
* value to name, only an absence to return to.
|
||||
*/
|
||||
@Target({ElementType.FIELD, ElementType.RECORD_COMPONENT})
|
||||
@Retention(RetentionPolicy.RUNTIME)
|
||||
public @interface DefaultsWhenEmpty {
|
||||
}
|
||||
|
|
@ -24,7 +24,8 @@ public class LLMProviderCatalogService {
|
|||
public List<LLMProviderMetadata> list() {
|
||||
return providers.values().stream()
|
||||
.map(provider -> new LLMProviderMetadata(provider.getName(), provider.requiresAuthorization(),
|
||||
provider.requiresEndpoint()))
|
||||
provider.requiresEndpoint(),
|
||||
provider.supportedParameters().stream().map(Enum::name).sorted().toList()))
|
||||
.filter(provider -> StringUtils.hasText(provider.name()))
|
||||
.distinct()
|
||||
.sorted(java.util.Comparator.comparing(LLMProviderMetadata::name, String.CASE_INSENSITIVE_ORDER))
|
||||
|
|
|
|||
|
|
@ -4,11 +4,22 @@
|
|||
|
||||
package it.cnr.isti.workflow.manager.llms;
|
||||
|
||||
import java.util.List;
|
||||
|
||||
/**
|
||||
* Public, non-sensitive capabilities used by every LLM selection UI.
|
||||
*
|
||||
* @param requiresEndpoint whether choosing this provider means the credential must also carry a
|
||||
* base URL - see {@link it.cnr.isti.workflow.manager.llms.providers.LLMProvider#requiresEndpoint()}.
|
||||
* @param supportedParameters which sampling knobs this provider actually applies, by
|
||||
* {@link ModelParameter} name. Every provider is offered the same five, so
|
||||
* without this the editor lets a value be set where it does nothing and the
|
||||
* run only says so afterwards, in a warning nobody was waiting for.
|
||||
*/
|
||||
public record LLMProviderMetadata(String name, boolean requiresCredential, boolean requiresEndpoint) {
|
||||
public record LLMProviderMetadata(String name, boolean requiresCredential, boolean requiresEndpoint,
|
||||
List<String> supportedParameters) {
|
||||
|
||||
public LLMProviderMetadata {
|
||||
supportedParameters = supportedParameters == null ? List.of() : List.copyOf(supportedParameters);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ import java.util.Set;
|
|||
import com.fasterxml.jackson.annotation.JsonIgnore;
|
||||
import com.fasterxml.jackson.annotation.JsonProperty;
|
||||
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.DefaultsWhenEmpty;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.UiDescription;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.UiLabel;
|
||||
import it.cnr.isti.workflow.manager.configurations.annotations.UiOrder;
|
||||
|
|
@ -43,6 +44,7 @@ public record ModelParameters(
|
|||
@UiDescription("Higher values make the output more varied. 0 makes it as repeatable as the model allows.")
|
||||
@DecimalMin("0.0") @DecimalMax("1.0")
|
||||
@JsonProperty(required = false)
|
||||
@DefaultsWhenEmpty
|
||||
Double temperature,
|
||||
|
||||
@UiOrder(20)
|
||||
|
|
@ -50,6 +52,7 @@ public record ModelParameters(
|
|||
@UiDescription("Nucleus sampling: consider only the most likely tokens adding up to this probability.")
|
||||
@DecimalMin("0.0") @DecimalMax("1.0")
|
||||
@JsonProperty(required = false)
|
||||
@DefaultsWhenEmpty
|
||||
Double topP,
|
||||
|
||||
@UiOrder(30)
|
||||
|
|
@ -57,6 +60,7 @@ public record ModelParameters(
|
|||
@UiDescription("Consider only this many candidate tokens at each step.")
|
||||
@Min(1)
|
||||
@JsonProperty(required = false)
|
||||
@DefaultsWhenEmpty
|
||||
Integer topK,
|
||||
|
||||
@UiOrder(40)
|
||||
|
|
@ -64,12 +68,14 @@ public record ModelParameters(
|
|||
@UiDescription("Upper bound on the length of the generated answer.")
|
||||
@Min(1)
|
||||
@JsonProperty(required = false)
|
||||
@DefaultsWhenEmpty
|
||||
Integer maxTokens,
|
||||
|
||||
@UiOrder(50)
|
||||
@UiLabel("Seed")
|
||||
@UiDescription("Fixes the randomness, so the same inputs give the same answer. Needed to tell a real change from model noise.")
|
||||
@JsonProperty(required = false)
|
||||
@DefaultsWhenEmpty
|
||||
Long seed) {
|
||||
|
||||
/** Which knobs this object actually sets. Empty when it asks for nothing. */
|
||||
|
|
|
|||
|
|
@ -629,6 +629,38 @@ public class BlocksControllerTest {
|
|||
assertFalse(prompt.has("x-ui-rows"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void onlyTheSamplingParametersDeclareTheyDefaultWhenEmpty() {
|
||||
// The editor used to offer "Use default" on every optional field, inferring it from the
|
||||
// field not being required - which says you may leave it blank, not that blank means
|
||||
// something. The claim is now made per field, and the sampling parameters are what it is
|
||||
// for: blank means the provider picks, and no typed number gets that state back.
|
||||
BlockConfigurationDescriptor descriptor = blocksController
|
||||
.getConfigurationDescriptorForType(LLMBlockType.TYPE);
|
||||
JsonNode schema = (JsonNode) descriptor.schema();
|
||||
|
||||
JsonNode parameters = findDefinition(schema, "ModelParameters");
|
||||
assertNotNull(parameters, "ModelParameters should be a schema definition");
|
||||
for (String knob : new String[] { "temperature", "topP", "topK", "maxTokens", "seed" }) {
|
||||
assertTrue(parameters.path("properties").path(knob).path("x-ui-defaults-when-empty").asBoolean(),
|
||||
knob + " should declare that empty means the provider decides");
|
||||
}
|
||||
|
||||
// prompt is optional too, and must no longer claim a default just for being optional.
|
||||
assertFalse(schema.path("properties").path("prompt").has("x-ui-defaults-when-empty"));
|
||||
assertFalse(schema.path("properties").path("skills").has("x-ui-defaults-when-empty"));
|
||||
}
|
||||
|
||||
private JsonNode findDefinition(JsonNode schema, String name) {
|
||||
for (String container : new String[] { "definitions", "$defs", "sharedDefinitions" }) {
|
||||
JsonNode found = schema.path(container).path(name);
|
||||
if (!found.isMissingNode() && found.has("properties")) {
|
||||
return found;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
@Test
|
||||
public void chatInteractionSchemaDeclaresUniqueInputNames() {
|
||||
BlockConfigurationDescriptor descriptor = blocksController
|
||||
|
|
|
|||
|
|
@ -0,0 +1,64 @@
|
|||
// SPDX-FileCopyrightText: 2025-2026 Lucio Lelii <lucio.lelii@isti.cnr.it> - ISTI-CNR
|
||||
// SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
// Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM.
|
||||
|
||||
package it.cnr.isti.workflow.manager.llms;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
|
||||
import java.util.EnumSet;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import it.cnr.isti.workflow.manager.llms.providers.LLMProvider;
|
||||
|
||||
/**
|
||||
* What the editor is told about a provider before anyone runs anything.
|
||||
*
|
||||
* <p>The capabilities exist so a choice that cannot work is not offered: every provider is handed
|
||||
* the same five sampling parameters, and the ones it ignores were only ever reported afterwards, in
|
||||
* an execution warning.
|
||||
*/
|
||||
class LLMProviderCatalogServiceTest {
|
||||
|
||||
private static LLMProvider provider(String name, Set<ModelParameter> parameters) {
|
||||
return new LLMProvider() {
|
||||
@Override
|
||||
public String getName() {
|
||||
return name;
|
||||
}
|
||||
|
||||
@Override
|
||||
public List<String> getRegisteredModels() {
|
||||
return List.of();
|
||||
}
|
||||
|
||||
@Override
|
||||
public String generate(String model, String prompt) {
|
||||
throw new UnsupportedOperationException("not exercised by this test");
|
||||
}
|
||||
|
||||
@Override
|
||||
public Set<ModelParameter> supportedParameters() {
|
||||
return parameters;
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
@Test
|
||||
void reportsWhichKnobsEachProviderActuallyApplies() {
|
||||
LLMProviderCatalogService catalog = new LLMProviderCatalogService(Map.of(
|
||||
"a", provider("Everything", EnumSet.allOf(ModelParameter.class)),
|
||||
"b", provider("NoSeed", EnumSet.of(ModelParameter.TEMPERATURE, ModelParameter.TOP_K))));
|
||||
|
||||
Map<String, LLMProviderMetadata> byName = catalog.list().stream()
|
||||
.collect(java.util.stream.Collectors.toMap(LLMProviderMetadata::name, metadata -> metadata));
|
||||
|
||||
assertEquals(List.of("MAX_TOKENS", "SEED", "TEMPERATURE", "TOP_K", "TOP_P"),
|
||||
byName.get("Everything").supportedParameters(), "sorted, so the payload is stable");
|
||||
assertEquals(List.of("TEMPERATURE", "TOP_K"), byName.get("NoSeed").supportedParameters());
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue