lelii
/
humainflow-service
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

feat: allow targeted repair on flows that contain containers 

isTargetedBlockRepairEligible previously bailed out to a full replan
whenever currentFlow had any containers at all, because
buildReusedPlanForTargetedRepair only reconstructed the top-level block
list - running it on a flow with containers would have silently
dropped them from the rebuilt plan.

buildReusedPlanForTargetedRepair now also reconstructs an
AssistantContainerPlan (operation KEEP, empty inner blocks) for every
container in currentFlow.flow().getContainers(), so containers survive
the targeted-repair path unchanged instead of disappearing. The
eligibility check still requires every error to be scoped to an
existing top-level block - an error scoped to a container (or anything
else) still falls back to a full replan, since a KEEP-only container
plan can't fix a container-internal problem.

Added a regression test: a flow with a container present has a
top-level block's overlong EndBlock.outcomeLabel fixed via a single
targeted BLOCK_CONFIG call (PLAN/CONNECTIONS both skipped), and the
container comes back byte-for-byte identical (same id, same inner
subflow).

Updated docs/assistant-completion-roadmap-2026-07-25.md to mark this
item done and note what's still open (container-internal errors are
still not targeted-fixable - that's the separate, larger "incremental
container diffing" item).
This commit is contained in:
Lucio Lelii 2026-07-25 16:59:15 +02:00
parent e482716068
commit ab1def92f1
2 changed files with 141 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
src/main/java/it/cnr/isti/workflow/manager/assistant/FlowAssistantService.java
View File

@ -1134,11 +1134,12 @@ public class FlowAssistantService {
* so the PLAN and CONNECTIONS phases can be skipped for this FIX round.
*/
private boolean isTargetedBlockRepairEligible(FlowCreateRequest currentFlow, List<ValidationError> errors) {
// Reusing the plan verbatim (buildReusedPlanForTargetedRepair) only reconstructs the
// block list - it would silently drop any existing containers. Bail out to the full
// repair path whenever containers are present rather than risk that.
if (errors == null || errors.isEmpty() || !hasCurrentFlowBlocks(currentFlow)
|| hasCurrentFlowContainers(currentFlow)) {
// buildReusedPlanForTargetedRepair carries existing containers forward as KEEP (untouched),
// so this stays eligible even when the flow has containers - as long as every error is
// still scoped to an existing *top-level block*. An error scoped to a container (or
// anything else) can't be fixed by a KEEP-only container plan, so it still disqualifies
// targeted repair and falls back to a full replan.
if (errors == null || errors.isEmpty() || !hasCurrentFlowBlocks(currentFlow)) {
return false;
}
Set<String> existingBlockIds = currentFlow.flow().getBlocks().stream()
@ -1172,7 +1173,21 @@ public class FlowAssistantService {
block.getName(),
erroringBlockIds.contains(block.getId()) ? PlanOperation.UPDATE.name() : PlanOperation.KEEP.name()))
.toList();
AssistantFlowPlan plan = new AssistantFlowPlan(currentFlow.name(), currentFlow.description(), blockPlans);
// isTargetedBlockRepairEligible only allows this path when every error is scoped to a
// top-level block, so any existing containers are always untouched here - just carried
// forward as KEEP so they aren't silently dropped from the rebuilt plan.
List<AssistantContainerPlan> containerPlans = hasCurrentFlowContainers(currentFlow)
? currentFlow.flow().getContainers().stream()
.map(container -> new AssistantContainerPlan(
container.getId(),
container.getType().getName(),
container.getName(),
PlanOperation.KEEP.name(),
List.of()))
.toList()
: List.of();
AssistantFlowPlan plan = new AssistantFlowPlan(currentFlow.name(), currentFlow.description(), blockPlans,
containerPlans);
return new ParsedPlan(plan, "Targeted repair: reconfiguring only the blocks flagged by validation errors.");
}

120
src/test/java/it/cnr/isti/workflow/manager/controllers/AssistantControllerTest.java
View File

@ -49,6 +49,9 @@ import it.cnr.isti.workflow.manager.blocks.types.HTTPServerCallBlockType;
import it.cnr.isti.workflow.manager.blocks.types.HumanInteractionBlockType;
import it.cnr.isti.workflow.manager.blocks.types.LLMBlockType;
import it.cnr.isti.workflow.manager.containers.Container;
import it.cnr.isti.workflow.manager.containers.configurations.GenericContainerConfiguration;
import it.cnr.isti.workflow.manager.containers.factories.GenericContainerFactory;
import it.cnr.isti.workflow.manager.containers.types.GenericContainerType;
import it.cnr.isti.workflow.manager.blocks.types.MCPAgentBlockType;
import it.cnr.isti.workflow.manager.flows.model.Connection;
import it.cnr.isti.workflow.manager.flows.model.FlowCreateRequest;
@ -89,6 +92,9 @@ public class AssistantControllerTest {
@MockitoBean
private InternalOllamaLLMProvider internalOllamaLLMProvider;
@Autowired
private GenericContainerFactory genericContainerFactory;
@Test
public void draftGeneratesValidFlow() {
mockAssistantResponses();
@ -440,6 +446,120 @@ public class AssistantControllerTest {
assertEquals("Done", fixedConfiguration.getOutcomeLabel());
}
@Test
public void fixSkipsPlanAndConnectionsForBlockScopedErrorWhenFlowHasContainers() {
LLMDescriptor llmDescriptor = LLMDescriptor.builder().provider("InternalOllama").model(MODEL).build();
LLMBlockConfiguration classifierConfiguration = LLMBlockConfiguration.builder()
.name("Ticket classifier")
.llmDescriptor(llmDescriptor)
.prompt("Classify the ticket: ${{ticket}}")
.build();
Block<LLMBlockType> classifierBlock = Block.<LLMBlockType>builder()
.specificConfiguration(classifierConfiguration)
.input(IODescriptor.of("ticket", IOType.TEXT))
.output(IODescriptor.of("response", IOType.TEXT))
.type(new LLMBlockType())
.build();
LLMBlockConfiguration innerConfiguration = LLMBlockConfiguration.builder()
.name("Summarize")
.llmDescriptor(llmDescriptor)
.prompt("Summarize: ${{classification}}")
.build();
Block<LLMBlockType> innerBlock = Block.<LLMBlockType>builder()
.specificConfiguration(innerConfiguration)
.input(IODescriptor.of("classification", IOType.TEXT))
.output(IODescriptor.of("response", IOType.TEXT))
.type(new LLMBlockType())
.build();
Container<GenericContainerType> container =
genericContainerFactory.create(GenericContainerConfiguration.builder()
.name("Review")
.subFlow(FlowData.builder().block(innerBlock).build())
.build());
EndBlockConfiguration endConfiguration = EndBlockConfiguration.builder()
.name("Done")
.outcomeCode("DONE")
.outcomeLabel("x".repeat(300))
.build();
Block<EndBlockType> endBlock = Block.<EndBlockType>builder()
.specificConfiguration(endConfiguration)
.input(IODescriptor.of("input", IOType.ANY))
.type(new EndBlockType())
.build();
FlowCreateRequest brokenFlow = new FlowCreateRequest(
"Ticket classification with review and terminal outcome",
"Classify a ticket, summarize it in a review container, then record a terminal outcome.",
FlowData.builder()
.block(classifierBlock)
.block(endBlock)
.container(container)
.connection(Connection.builder()
.sourceId(classifierBlock.getId())
.sourceName("response")
.targetId(container.getId())
.targetName("classification")
.build())
.connection(Connection.builder()
.sourceId(container.getId())
.sourceName("response")
.targetId(endBlock.getId())
.targetName("input")
.build())
.build());
// Only mocking BLOCK_CONFIG for the flagged (top-level, non-container) block: if targeted
// repair incorrectly bailed out to a full replan because the flow has a container, or
// dropped the container while rebuilding the plan, this mock's fallback would throw or
// the container assertions below would fail.
Answer<String> answer = invocation -> {
String prompt = invocation.getArgument(1, String.class);
if (prompt.contains("TASK: BLOCK_CONFIG")) {
return TestAssistantResponses.wrap(java.util.Map.of(
"rationale", "Shortened the overlong outcome label.",
"block", java.util.Map.of(
"blockId", endBlock.getId(),
"name", "Done",
"config", java.util.Map.of(
"outcomeCode", "DONE",
"outcomeLabel", "Done"))));
}
throw new IllegalStateException("Unexpected assistant prompt (expected only BLOCK_CONFIG):\n" + prompt);
};
Mockito.when(internalOllamaLLMProvider.generate(Mockito.eq(MODEL), Mockito.anyString())).thenAnswer(answer);
Mockito.when(internalOllamaLLMProvider.generateJson(Mockito.eq(MODEL), Mockito.anyString())).thenAnswer(answer);
java.util.List<ValidationError> validationErrors = java.util.List.of(new ValidationError(
ValidationErrorCode.VALIDATION_ERROR, "block", endBlock.getId(),
"specificConfiguration.outcomeLabel", "size must be between 0 and 255"));
AssistantFlowResponse response = assistantController.fix(
new AssistantFixRequest("Fix the overlong outcome label", brokenFlow, validationErrors, MODEL, 1));
assertTrue(response.valid(), () -> "Unexpected validation errors: " + response.validationErrors());
assertTrue(response.validationErrors().isEmpty());
assertEquals(2, response.flow().flow().getBlocks().size());
assertEquals(1, response.flow().flow().getContainers().size());
assertEquals(2, response.flow().flow().getConnections().size());
// The container must survive untouched: same id, same inner subflow.
Container<?> resultContainer = response.flow().flow().getContainers().getFirst();
assertEquals(container.getId(), resultContainer.getId());
assertEquals(1, resultContainer.getSpecificConfiguration().getSubFlow().getBlocks().size());
assertEquals("Summarize",
resultContainer.getSpecificConfiguration().getSubFlow().getBlocks().getFirst().getName());
Block<?> fixedEndBlock = response.flow().flow().getBlocks().stream()
.filter(block -> "Done".equals(block.getName()))
.findFirst()
.orElseThrow();
EndBlockConfiguration fixedConfiguration = (EndBlockConfiguration) fixedEndBlock.getSpecificConfiguration();
assertEquals("Done", fixedConfiguration.getOutcomeLabel());
}
@Test
public void explainReturnsNarrativeText() {
mockAssistantResponses();