diff --git a/src/main/java/it/cnr/isti/workflow/manager/assistant/FlowAssistantService.java b/src/main/java/it/cnr/isti/workflow/manager/assistant/FlowAssistantService.java index 2dd9208..3843235 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/assistant/FlowAssistantService.java +++ b/src/main/java/it/cnr/isti/workflow/manager/assistant/FlowAssistantService.java @@ -1152,6 +1152,7 @@ public class FlowAssistantService { normalizedConfig.put("name", defaultIfBlank(draft.name(), defaultIfBlank(blockPlan.purpose(), blockPlan.blockType()))); injectSystemManagedFields(normalizedConfig, descriptor, model); ensureRequiredTextDefaults(normalizedConfig, descriptor, blockPlan); + normalizeHumanDecisionOptions(normalizedConfig, descriptor); normalizeHttpServerCallAuthorization(normalizedConfig, blockPlan); normalizeMcpAgentSharedMemory(normalizedConfig, blockPlan, model, requireSharedMemorySemantics); ensureSequentialInputPlaceholder(normalizedConfig, descriptor, blockPlan, blockIndex, blockCount); @@ -1340,6 +1341,44 @@ public class FlowAssistantService { } } + /** + * HumanDecisionOption is (name, label): name is the routing key / branch output, label the + * display text. Models routinely emit "value" (or only "label") instead of "name", leaving name + * null - which produces null-named branch outputs. Fill a missing option name from its "value" + * (the intended routing key) or from a slug of its "label", so the flow is valid and each branch + * has a real output name. + */ + private void normalizeHumanDecisionOptions(ObjectNode config, + BlockCatalogService.AssistantPromptBlockDescriptor descriptor) { + if (!isConfigurationType(descriptor.configurationType(), "HumanDecisionBlockConfiguration")) { + return; + } + if (!(config.get("options") instanceof ArrayNode options)) { + return; + } + for (JsonNode option : options) { + if (!(option instanceof ObjectNode optionObject) || hasTextValue(optionObject.get("name"))) { + continue; + } + String derived = hasTextValue(optionObject.get("value")) + ? textOrNull(optionObject.get("value")).trim() + : hasTextValue(optionObject.get("label")) + ? slugifyOptionName(textOrNull(optionObject.get("label"))) + : null; + if (derived != null && !derived.isBlank()) { + optionObject.put("name", derived); + } + } + } + + private String slugifyOptionName(String label) { + String slug = label.trim().toLowerCase(Locale.ROOT).replaceAll("[^a-z0-9]+", "-").replaceAll("(^-+|-+$)", ""); + if (slug.isBlank()) { + return "option"; + } + return Character.isLetter(slug.charAt(0)) ? slug : "opt-" + slug; + } + private void removeSystemManagedFields(ObjectNode config) { config.remove(List.of("provider", "model", "llmDescriptor", "ids", "inputs", "outputs", "skills")); } diff --git a/src/main/java/it/cnr/isti/workflow/manager/ios/IODescriptor.java b/src/main/java/it/cnr/isti/workflow/manager/ios/IODescriptor.java index 00ab3d4..82ce70e 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/ios/IODescriptor.java +++ b/src/main/java/it/cnr/isti/workflow/manager/ios/IODescriptor.java @@ -81,7 +81,7 @@ public class IODescriptor { @Override public int hashCode() { - return name.hashCode(); + return java.util.Objects.hashCode(name); } @Override @@ -91,6 +91,6 @@ public class IODescriptor { if (obj == null || getClass() != obj.getClass()) return false; IODescriptor other = (IODescriptor) obj; - return name.equals(other.name); + return java.util.Objects.equals(name, other.name); } } diff --git a/src/test/java/it/cnr/isti/workflow/manager/controllers/AssistantControllerTest.java b/src/test/java/it/cnr/isti/workflow/manager/controllers/AssistantControllerTest.java index 7a7f8e9..b9a5ebb 100644 --- a/src/test/java/it/cnr/isti/workflow/manager/controllers/AssistantControllerTest.java +++ b/src/test/java/it/cnr/isti/workflow/manager/controllers/AssistantControllerTest.java @@ -2759,6 +2759,51 @@ public class AssistantControllerTest { assertEquals("Help the user plan a trip", configuration.getGoalDescription()); } + @Test + public void draftNormalizesHumanDecisionOptionNamesFromValueOrLabel() { + // Models routinely emit HumanDecision options as {label, value} instead of {name, label}, + // leaving the routing key (name) null - which used to make null-named branch outputs and a + // 500. The backend now fills name from value (or a slug of label) so the flow is valid. + Answer answer = invocation -> { + String prompt = invocation.getArgument(1, String.class); + if (prompt.contains("TASK: PLAN")) { + return TestAssistantResponses.wrap(java.util.Map.of("rationale", "One review decision.", + "plan", java.util.Map.of("name", "Review", "description", "desc", + "blocks", java.util.List.of(java.util.Map.of("blockId", "b1", + "blockType", "HumanDecisionBlock", "purpose", "Approve or reject"))))); + } + if (prompt.contains("TASK: BLOCK_CONFIG")) { + return TestAssistantResponses.wrap(java.util.Map.of("rationale", "decision config", + "block", java.util.Map.of("blockId", "b1", "name", "review", + "config", java.util.Map.of( + "question", "Is it ready for approval?", + "options", java.util.List.of( + java.util.Map.of("label", "Approve", "value", "yes"), + java.util.Map.of("label", "Reject")), // only label, no value + "rationaleRequired", true)))); + } + throw new IllegalStateException("Unexpected assistant prompt:\n" + prompt); + }; + Mockito.when(internalOllamaLLMProvider.generate(Mockito.eq(MODEL), Mockito.anyString())).thenAnswer(answer); + Mockito.when(internalOllamaLLMProvider.generateJson(Mockito.eq(MODEL), Mockito.anyString())).thenAnswer(answer); + + AssistantFlowResponse response = assistantController.draft( + new AssistantGenerationRequest("a human review that approves or rejects", MODEL, 1)); + + assertNotNull(response); + assertTrue(response.valid(), () -> "Unexpected validation errors: " + response.validationErrors()); + it.cnr.isti.workflow.manager.blocks.configurations.HumanDecisionBlockConfiguration configuration = + (it.cnr.isti.workflow.manager.blocks.configurations.HumanDecisionBlockConfiguration) + response.flow().flow().getBlocks().getFirst().getSpecificConfiguration(); + java.util.List optionNames = configuration.getOptions().stream() + .map(it.cnr.isti.workflow.manager.blocks.configurations.HumanDecisionOption::name).toList(); + // "yes" taken from value; "reject" slugged from the label (no value given). + assertEquals(java.util.List.of("yes", "reject"), optionNames); + // The branch outputs are named accordingly (never null). + assertTrue(response.flow().flow().getBlocks().getFirst().getOutputs().stream() + .allMatch(output -> output.getName() != null && !output.getName().isBlank())); + } + static class TestAssistantResponses { private static final String PROVIDER = "InternalOllama";