From a5d94a2e55595ca670bcac4402d6b9aac076233b Mon Sep 17 00:00:00 2001 From: Lucio Lelii Date: Fri, 24 Jul 2026 22:04:15 +0200 Subject: [PATCH] fix: tolerant parsing of the simulator's HumanDecision choice Simulated HumanDecision required the LLM's "CHOICE:" line to be exactly a configured option name. But the simulate prompt lists options as "- name: Label", so models - including strong instruct ones (qwen2.5:14b, etc.), not just gemma:7b - routinely echo the whole "name: Label" line, e.g. "red: Red light" or "existing: Existing position". That was rejected with HUMAN_DECISION_INVALID_CHOICE, breaking simulated runs of any flow with human decisions. resolveSimulatedChoice() now accepts, in order: an exact option name, the token before the first ':' (the echoed "name: Label" case), or the option label. Only ':' is treated as a separator - never '-' - so hyphenated option names like "not-red"/"assessment-not-required" are never truncated. Unknown choices still return null and raise the same error. Added a direct unit test (HumanDecisionSimulatedChoiceTest) covering exact name, echoed name:label, hyphenated names, label text and rejection. Also narrowed the flat-flow Jensen smoke test to exclude the new container-grouped variant (different top-level shape). Co-Authored-By: Claude Sonnet 5 --- .../blocks/HumanDecisionExecutor.java | 47 ++++++++++++++-- .../HumanDecisionSimulatedChoiceTest.java | 55 +++++++++++++++++++ .../flows/FlowImportComponentTest.java | 4 ++ 3 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 src/test/java/it/cnr/isti/workflow/manager/executions/executors/blocks/HumanDecisionSimulatedChoiceTest.java diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/HumanDecisionExecutor.java b/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/HumanDecisionExecutor.java index 5d0719a..37ee3fb 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/HumanDecisionExecutor.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/HumanDecisionExecutor.java @@ -103,12 +103,11 @@ public class HumanDecisionExecutor implements BlockExecutor name.equalsIgnoreCase(resolvedChoice)) - .findFirst() - .orElseThrow(() -> new NodeDecisionException("HUMAN_DECISION_INVALID_CHOICE", - "Simulator returned unsupported HumanDecision choice: " + resolvedChoice)); + String choice = resolveSimulatedChoice(configuration, rawChoice); + if (choice == null) { + throw new NodeDecisionException("HUMAN_DECISION_INVALID_CHOICE", + "Simulator returned unsupported HumanDecision choice: " + resolvedChoice); + } if (configuration.isRationaleRequired() && !StringUtils.hasText(rationale)) { throw new NodeDecisionException("HUMAN_DECISION_SIMULATION_FAILED", "Simulator did not provide a required rationale"); @@ -127,6 +126,42 @@ public class HumanDecisionExecutor implements BlockExecutor + * The simulate prompt lists options as "- name: Label", so models (even strong ones) + * routinely echo the whole "name: Label" line rather than the bare name. This accepts + * an exact option name, the token before the first ':' (the echoed "name: Label" case), + * or the option label. Returns null if nothing matches. Option names may contain '-' + * (e.g. "not-red"), so only ':' is treated as a separator, never '-'. + */ + static String resolveSimulatedChoice(HumanDecisionBlockConfiguration configuration, String rawChoice) { + String candidate = rawChoice == null ? "" : rawChoice.strip(); + if (candidate.isEmpty()) { + return null; + } + String beforeColon = candidate.contains(":") + ? candidate.substring(0, candidate.indexOf(':')).strip() + : candidate; + for (HumanDecisionOption option : configuration.getOptions()) { + if (option.name().equalsIgnoreCase(candidate)) { + return option.name(); + } + } + for (HumanDecisionOption option : configuration.getOptions()) { + if (option.name().equalsIgnoreCase(beforeColon)) { + return option.name(); + } + } + for (HumanDecisionOption option : configuration.getOptions()) { + if (option.label() != null + && (option.label().equalsIgnoreCase(candidate) || option.label().equalsIgnoreCase(beforeColon))) { + return option.name(); + } + } + return null; + } + @Override public InteractionResult interact(Block block, List inputs, Map interaction, Map partialResults, Map authorizations, diff --git a/src/test/java/it/cnr/isti/workflow/manager/executions/executors/blocks/HumanDecisionSimulatedChoiceTest.java b/src/test/java/it/cnr/isti/workflow/manager/executions/executors/blocks/HumanDecisionSimulatedChoiceTest.java new file mode 100644 index 0000000..1cd1cda --- /dev/null +++ b/src/test/java/it/cnr/isti/workflow/manager/executions/executors/blocks/HumanDecisionSimulatedChoiceTest.java @@ -0,0 +1,55 @@ +package it.cnr.isti.workflow.manager.executions.executors.blocks; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +import java.util.List; + +import org.junit.jupiter.api.Test; + +import it.cnr.isti.workflow.manager.blocks.configurations.HumanDecisionBlockConfiguration; +import it.cnr.isti.workflow.manager.blocks.configurations.HumanDecisionOption; + +class HumanDecisionSimulatedChoiceTest { + + private HumanDecisionBlockConfiguration config() { + return HumanDecisionBlockConfiguration.builder() + .name("decision") + .question("Pick a light") + .options(List.of( + new HumanDecisionOption("red", "Red light"), + new HumanDecisionOption("not-red", "No red flag"))) + .build(); + } + + @Test + void acceptsExactOptionName() { + assertEquals("red", HumanDecisionExecutor.resolveSimulatedChoice(config(), "red")); + assertEquals("not-red", HumanDecisionExecutor.resolveSimulatedChoice(config(), "NOT-RED")); + } + + @Test + void acceptsEchoedNameColonLabel() { + // Models routinely echo the whole "- name: Label" prompt line. + assertEquals("red", HumanDecisionExecutor.resolveSimulatedChoice(config(), "red: Red light")); + assertEquals("not-red", HumanDecisionExecutor.resolveSimulatedChoice(config(), "not-red: No red flag")); + } + + @Test + void doesNotSplitHyphenatedNames() { + // "not-red" must not be truncated to "not" by treating '-' as a separator. + assertEquals("not-red", HumanDecisionExecutor.resolveSimulatedChoice(config(), "not-red")); + } + + @Test + void acceptsLabelText() { + assertEquals("red", HumanDecisionExecutor.resolveSimulatedChoice(config(), "Red light")); + } + + @Test + void rejectsUnrelatedChoice() { + assertNull(HumanDecisionExecutor.resolveSimulatedChoice(config(), "maybe")); + assertNull(HumanDecisionExecutor.resolveSimulatedChoice(config(), "")); + assertNull(HumanDecisionExecutor.resolveSimulatedChoice(config(), null)); + } +} diff --git a/src/test/java/it/cnr/isti/workflow/manager/flows/FlowImportComponentTest.java b/src/test/java/it/cnr/isti/workflow/manager/flows/FlowImportComponentTest.java index 70c7c69..49ffbb3 100644 --- a/src/test/java/it/cnr/isti/workflow/manager/flows/FlowImportComponentTest.java +++ b/src/test/java/it/cnr/isti/workflow/manager/flows/FlowImportComponentTest.java @@ -101,8 +101,12 @@ public class FlowImportComponentTest { List flows = ObjectMapperHolder.mapper.readValue( bundledFlows.toFile(), ObjectMapperHolder.mapper.getTypeFactory().constructCollectionType(List.class, ImportedFlow.class)); + // The container-grouped variant has a different top-level shape (a GenericContainer + // first node that suspends into WAITING_FOR_SUBFLOW, not a top-level HumanInteractionBlock), + // so it is covered by its own tests, not this flat-flow smoke test. List jensenFlows = flows.stream() .filter(flow -> flow.name().startsWith("Jensen Recruitment Process - ")) + .filter(flow -> !flow.name().contains("(Containerized)")) .toList(); assertEquals(2, jensenFlows.size());