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) <noreply@anthropic.com>
This commit is contained in:
parent
5a855c9598
commit
d14da5d5f3
|
|
@ -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<LoopConta
|
|||
super(name, subFlow);
|
||||
this.guardSubFlow = guardSubFlow == null ? FlowData.builder().build() : guardSubFlow;
|
||||
this.feedbackInput = feedbackInput;
|
||||
this.maxIterations = maxIterations == null ? 10 : maxIterations;
|
||||
this.maxIterations = maxIterations == null ? LoopLimits.DEFAULT_MAX_ITERATIONS : maxIterations;
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
@ -88,7 +89,7 @@ public class LoopContainerConfiguration extends ContainerConfiguration<LoopConta
|
|||
FlowData.builder().build(),
|
||||
FlowData.builder().build(),
|
||||
null,
|
||||
10);
|
||||
LoopLimits.DEFAULT_MAX_ITERATIONS);
|
||||
}
|
||||
|
||||
@AssertTrue(message = "maxIterations must be greater than zero")
|
||||
|
|
|
|||
|
|
@ -10,10 +10,7 @@ import java.util.Set;
|
|||
import java.util.stream.Collectors;
|
||||
|
||||
import it.cnr.isti.workflow.manager.blocks.Block;
|
||||
import it.cnr.isti.workflow.manager.blocks.configurations.ConditionalBlockConfiguration;
|
||||
import it.cnr.isti.workflow.manager.blocks.configurations.EndBlockConfiguration;
|
||||
import it.cnr.isti.workflow.manager.blocks.configurations.HumanDecisionBlockConfiguration;
|
||||
import it.cnr.isti.workflow.manager.blocks.configurations.SwitchBlockConfiguration;
|
||||
import it.cnr.isti.workflow.manager.containers.Container;
|
||||
import it.cnr.isti.workflow.manager.executions.ExecutionEventLogger;
|
||||
import it.cnr.isti.workflow.manager.executions.ContainerExecutionContext;
|
||||
|
|
@ -239,10 +236,7 @@ public final class NodeExecutors {
|
|||
|
||||
private static NodeExecutionResult nodeExecutionResult(FlowNode node, List<Input> inputs,
|
||||
Map<String, Object> 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<String> declaredOutputs = node.getOutputs().stream()
|
||||
.map(output -> output.getName())
|
||||
.collect(Collectors.toCollection(java.util.LinkedHashSet::new));
|
||||
|
|
|
|||
|
|
@ -0,0 +1,27 @@
|
|||
// 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.flows.model;
|
||||
|
||||
/**
|
||||
* How many times a flow may go round, wherever the going round is drawn.
|
||||
*
|
||||
* <p>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() {
|
||||
}
|
||||
}
|
||||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<ValidFlowStructure
|
|||
}
|
||||
|
||||
private List<String> 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) {
|
||||
|
|
|
|||
|
|
@ -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<String> 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<EndBlockType> original = end();
|
||||
|
|
|
|||
Loading…
Reference in New Issue