From d14da5d5f356deb26bfefa993ee86a9ec2b56e4f Mon Sep 17 00:00:00 2001 From: Lucio Lelii Date: Fri, 25 Sep 2026 11:08:43 +0200 Subject: [PATCH] Let a block type declare that it routes exclusively Which blocks pick one output and leave the rest as branches not taken was written out twice by name: in branch validation and in the engine's not-selected marking. It is now a capability the type declares, so a routing block added later is treated as one without either learning its name - and loops can use the same capability to find their guard. The loop container's default of ten iterations moves to a shared constant, for connections that lead back to reuse. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../LoopContainerConfiguration.java | 5 ++-- .../executions/executors/NodeExecutors.java | 8 +----- .../manager/flows/model/LoopLimits.java | 27 +++++++++++++++++++ .../capabilities/NodeTypeCapabilities.java | 17 +++++++----- .../flows/validation/FlowDataValidator.java | 24 +++++------------ .../NodeTypeCapabilitiesIntegrationTest.java | 19 +++++++++++++ 6 files changed, 68 insertions(+), 32 deletions(-) create mode 100644 src/main/java/it/cnr/isti/workflow/manager/flows/model/LoopLimits.java diff --git a/src/main/java/it/cnr/isti/workflow/manager/containers/configurations/LoopContainerConfiguration.java b/src/main/java/it/cnr/isti/workflow/manager/containers/configurations/LoopContainerConfiguration.java index 2df23d1..3fd322e 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/containers/configurations/LoopContainerConfiguration.java +++ b/src/main/java/it/cnr/isti/workflow/manager/containers/configurations/LoopContainerConfiguration.java @@ -19,6 +19,7 @@ import it.cnr.isti.workflow.manager.configurations.annotations.UiOrder; import it.cnr.isti.workflow.manager.containers.iresolvers.ContainerFlowInterfaceResolver; import it.cnr.isti.workflow.manager.containers.types.LoopContainerType; import it.cnr.isti.workflow.manager.flows.model.FlowData; +import it.cnr.isti.workflow.manager.flows.model.LoopLimits; import jakarta.validation.Valid; import jakarta.validation.constraints.AssertTrue; import jakarta.validation.constraints.Min; @@ -74,7 +75,7 @@ public class LoopContainerConfiguration extends ContainerConfiguration inputs, Map outputs) { - if (node instanceof Block block - && (block.getSpecificConfiguration() instanceof ConditionalBlockConfiguration - || block.getSpecificConfiguration() instanceof SwitchBlockConfiguration - || block.getSpecificConfiguration() instanceof HumanDecisionBlockConfiguration)) { + if (node instanceof Block block && block.getType().getCapabilities().routesExclusively()) { Set declaredOutputs = node.getOutputs().stream() .map(output -> output.getName()) .collect(Collectors.toCollection(java.util.LinkedHashSet::new)); diff --git a/src/main/java/it/cnr/isti/workflow/manager/flows/model/LoopLimits.java b/src/main/java/it/cnr/isti/workflow/manager/flows/model/LoopLimits.java new file mode 100644 index 0000000..8666d2f --- /dev/null +++ b/src/main/java/it/cnr/isti/workflow/manager/flows/model/LoopLimits.java @@ -0,0 +1,27 @@ +// SPDX-FileCopyrightText: 2025-2026 Lucio Lelii - 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.flows.model; + +/** + * How many times a flow may go round, wherever the going round is drawn. + * + *

A flow can repeat work in two ways - a loop container, or a connection that leads back to an + * earlier block - and a person who has learned one default should not meet a different one in + * the other. Both read the default from here, so it cannot drift apart. + */ +public final class LoopLimits { + + /** Iterations allowed when the author has not said how many. */ + public static final int DEFAULT_MAX_ITERATIONS = 10; + + /** + * The most a connection leading back may allow. A loop that needs more than this is not + * converging; it is the kind of run the limit exists to stop. + */ + public static final int MAX_BACK_EDGE_ITERATIONS = 100; + + private LoopLimits() { + } +} diff --git a/src/main/java/it/cnr/isti/workflow/manager/flows/model/capabilities/NodeTypeCapabilities.java b/src/main/java/it/cnr/isti/workflow/manager/flows/model/capabilities/NodeTypeCapabilities.java index 338870c..1b60633 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/flows/model/capabilities/NodeTypeCapabilities.java +++ b/src/main/java/it/cnr/isti/workflow/manager/flows/model/capabilities/NodeTypeCapabilities.java @@ -11,31 +11,36 @@ public record NodeTypeCapabilities( boolean allowsIncomingConnections, boolean allowsOutgoingConnections, boolean canDependOnOtherNodes, - boolean canHaveDependentNodes) { + boolean canHaveDependentNodes, + // Produces exactly one of its outputs per run, and the others are the branches not taken. + // Declared by the type rather than inferred from which block it is, so that a routing + // block added later is treated as one - by branch validation, by the engine and by loops - + // without any of them learning its name. + boolean routesExclusively) { public NodeTypeCapabilities { visualRole = visualRole == null ? NodeVisualRole.ACTIVITY : visualRole; } public static NodeTypeCapabilities activity() { - return new NodeTypeCapabilities(NodeVisualRole.ACTIVITY, false, true, true, true, true, true); + return new NodeTypeCapabilities(NodeVisualRole.ACTIVITY, false, true, true, true, true, true, false); } public static NodeTypeCapabilities decision() { - return new NodeTypeCapabilities(NodeVisualRole.DECISION, false, true, true, true, true, true); + return new NodeTypeCapabilities(NodeVisualRole.DECISION, false, true, true, true, true, true, true); } public static NodeTypeCapabilities branchRejoin() { - return new NodeTypeCapabilities(NodeVisualRole.BRANCH_REJOIN, false, true, true, true, true, true); + return new NodeTypeCapabilities(NodeVisualRole.BRANCH_REJOIN, false, true, true, true, true, true, false); } public static NodeTypeCapabilities end() { - return new NodeTypeCapabilities(NodeVisualRole.END, true, false, true, false, false, false); + return new NodeTypeCapabilities(NodeVisualRole.END, true, false, true, false, false, false, false); } public static NodeTypeCapabilities container() { // biasAnnotationsAllowed = false: containers are not bias-annotatable; // bias lives only on the inner subflow nodes (activated via includeSubflow). - return new NodeTypeCapabilities(NodeVisualRole.CONTAINER, false, false, true, true, true, true); + return new NodeTypeCapabilities(NodeVisualRole.CONTAINER, false, false, true, true, true, true, false); } } diff --git a/src/main/java/it/cnr/isti/workflow/manager/flows/validation/FlowDataValidator.java b/src/main/java/it/cnr/isti/workflow/manager/flows/validation/FlowDataValidator.java index 8466795..220c44f 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/flows/validation/FlowDataValidator.java +++ b/src/main/java/it/cnr/isti/workflow/manager/flows/validation/FlowDataValidator.java @@ -24,11 +24,7 @@ import it.cnr.isti.workflow.manager.blocks.configurations.BranchRejoinBlockConfi import it.cnr.isti.workflow.manager.blocks.configurations.LLMBlockConfiguration; import it.cnr.isti.workflow.manager.blocks.configurations.SwitchBlockConfiguration; import it.cnr.isti.workflow.manager.blocks.factories.BlockFactory; -import it.cnr.isti.workflow.manager.blocks.factories.ConditionalBlockFactory; -import it.cnr.isti.workflow.manager.blocks.types.ConditionalBlockType; import it.cnr.isti.workflow.manager.blocks.types.BranchRejoinBlockType; -import it.cnr.isti.workflow.manager.blocks.types.HumanDecisionBlockType; -import it.cnr.isti.workflow.manager.blocks.types.SwitchBlockType; import it.cnr.isti.workflow.manager.containers.Container; import it.cnr.isti.workflow.manager.containers.configurations.ContainerConfiguration; import it.cnr.isti.workflow.manager.containers.configurations.GenericContainerConfiguration; @@ -691,20 +687,14 @@ public class FlowDataValidator implements ConstraintValidator exclusiveRoutingOutputs(Block block) { - if (ConditionalBlockType.TYPE.equals(block.getType().getName())) { - return List.of(ConditionalBlockFactory.TRUE_OUTPUT, ConditionalBlockFactory.FALSE_OUTPUT); + // Every output of a type that routes exclusively is one of its branches: it is the type that + // says so, not a list of the routing blocks that happen to exist today. + if (!block.getType().getCapabilities().routesExclusively()) { + return List.of(); } - if (SwitchBlockType.TYPE.equals(block.getType().getName())) { - return block.getOutputs().stream() - .map(IODescriptor::getName) - .toList(); - } - if (HumanDecisionBlockType.TYPE.equals(block.getType().getName())) { - return block.getOutputs().stream() - .map(IODescriptor::getName) - .toList(); - } - return List.of(); + return block.getOutputs().stream() + .map(IODescriptor::getName) + .toList(); } private boolean isBranchRejoin(FlowNode node) { diff --git a/src/test/java/it/cnr/isti/workflow/manager/flows/validation/NodeTypeCapabilitiesIntegrationTest.java b/src/test/java/it/cnr/isti/workflow/manager/flows/validation/NodeTypeCapabilitiesIntegrationTest.java index b9d0f27..d8d7ce1 100644 --- a/src/test/java/it/cnr/isti/workflow/manager/flows/validation/NodeTypeCapabilitiesIntegrationTest.java +++ b/src/test/java/it/cnr/isti/workflow/manager/flows/validation/NodeTypeCapabilitiesIntegrationTest.java @@ -10,6 +10,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -21,7 +22,9 @@ import it.cnr.isti.workflow.manager.blocks.configurations.EndBlockConfiguration; import it.cnr.isti.workflow.manager.blocks.configurations.HumanInteractiveBlockConfiguration; import it.cnr.isti.workflow.manager.blocks.factories.EndBlockFactory; import it.cnr.isti.workflow.manager.blocks.factories.HumanInteractiveBlockFactory; +import it.cnr.isti.workflow.manager.blocks.types.ConditionalBlockType; import it.cnr.isti.workflow.manager.blocks.types.EndBlockType; +import it.cnr.isti.workflow.manager.blocks.types.SwitchBlockType; import it.cnr.isti.workflow.manager.blocks.types.BranchRejoinBlockType; import it.cnr.isti.workflow.manager.blocks.types.HumanDecisionBlockType; import it.cnr.isti.workflow.manager.blocks.types.HumanInteractionBlockType; @@ -91,6 +94,22 @@ class NodeTypeCapabilitiesIntegrationTest { assertEquals(container, compactContainer); } + @Test + void exactlyTheTypesThatPickOneOutputDeclareThatTheyRouteExclusively() { + // Branch validation, the engine's not-selected marking and loop guards all read this one + // flag. Pinning which types set it is what keeps the refactor away from those three + // hard-coded lists honest - and makes the next routing block a one-line declaration. + Set routing = blocksController.getConfigurationCatalog().descriptors().stream() + .filter(descriptor -> descriptor.capabilities().routesExclusively()) + .map(descriptor -> descriptor.type()) + .collect(Collectors.toSet()); + + assertEquals(Set.of(ConditionalBlockType.TYPE, SwitchBlockType.TYPE, HumanDecisionBlockType.TYPE), routing); + assertTrue(containersController.getTypeCatalog().descriptors().stream() + .noneMatch(descriptor -> descriptor.capabilities().routesExclusively()), + "a container's outputs depend on its subflow, so none promises to produce exactly one"); + } + @Test void endBlockRejectsBiasAnnotations() { Block original = end();