diff --git a/src/main/java/it/cnr/isti/workflow/manager/assistant/FlowAssistantPromptService.java b/src/main/java/it/cnr/isti/workflow/manager/assistant/FlowAssistantPromptService.java index d5ba8a7..79bf72d 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/assistant/FlowAssistantPromptService.java +++ b/src/main/java/it/cnr/isti/workflow/manager/assistant/FlowAssistantPromptService.java @@ -8,6 +8,9 @@ import org.springframework.stereotype.Service; import it.cnr.isti.workflow.manager.app.ObjectMapperHolder; import it.cnr.isti.workflow.manager.flows.model.FlowCreateRequest; import it.cnr.isti.workflow.manager.flows.validation.ValidationError; +import tools.jackson.databind.JsonNode; +import tools.jackson.databind.node.ArrayNode; +import tools.jackson.databind.node.ObjectNode; @Service public class FlowAssistantPromptService { @@ -470,7 +473,40 @@ public class FlowAssistantPromptService { if (flow == null || flow.flow() == null) { return "(none)"; } - return toJson(flow); + JsonNode tree = ObjectMapperHolder.mapper.valueToTree(flow); + redactLoopGuardSubFlows(tree); + return tree.toPrettyString(); + } + + /** + * A LoopContainer's guardSubFlow is a rigid scaffold the backend builds deterministically from + * guardCondition (see FlowAssistantService#buildLoopGuardSubFlow) - the assistant never authors + * or edits it. Showing its real block ids/names (e.g. "c1-expose-feedback") in the "Current + * flow" context invites the model to wire connections directly to/from them, which can never + * resolve at the model's scope and is silently dropped - but wastes a repair round doing so and + * can crowd out the legitimate fix the round actually needed. Redact it to a short marker so the + * model never sees it as something to reference or edit. + */ + private void redactLoopGuardSubFlows(JsonNode node) { + if (node == null) { + return; + } + if (node.isObject()) { + ObjectNode object = (ObjectNode) node; + JsonNode specificConfiguration = object.get("specificConfiguration"); + if (specificConfiguration != null && specificConfiguration.isObject() + && specificConfiguration.get("guardSubFlow") != null) { + ((ObjectNode) specificConfiguration).put("guardSubFlow", + "(backend-managed guard mechanism - not shown, do not reference or connect to it)"); + } + for (java.util.Map.Entry entry : object.properties()) { + redactLoopGuardSubFlows(entry.getValue()); + } + } else if (node.isArray()) { + for (JsonNode element : (ArrayNode) node) { + redactLoopGuardSubFlows(element); + } + } } private String summarizeErrors(List errors) { 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 b089089..49703e3 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 @@ -2888,6 +2888,96 @@ public class AssistantControllerTest { assertNotEquals("previous_response", configuration.getFeedbackInput()); } + @Test + public void fixPromptRedactsLoopGuardSubFlowInsteadOfExposingItForEditing() { + // Live incident: in FIX mode the model was shown the full "Current flow" JSON, including a + // LoopContainer's guardSubFlow - the deterministic, backend-built scaffold it never authors + // (see buildLoopGuardSubFlow). Seeing real internal block names like "c1-expose-feedback" in + // that context, the model tried to wire connections directly to/from them. Those connections + // can never resolve at the model's scope (they get silently dropped), but the round is wasted + // instead of fixing the actual reported error. The guardSubFlow must be redacted from what + // the model sees. + Answer draftAnswer = invocation -> { + String prompt = invocation.getArgument(1, String.class); + if (prompt.contains("TASK: PLAN")) { + return TestAssistantResponses.wrap(java.util.Map.of( + "rationale", "Revise the draft repeatedly until it passes review - a loop.", + "plan", java.util.Map.of( + "name", "Iterative revision", + "description", "Revise a draft until it has no issues.", + "blocks", java.util.List.of(), + "containers", java.util.List.of( + java.util.Map.of( + "containerId", "c1", + "containerType", "LoopContainer", + "purpose", "Revise until clean", + "operation", "ADD", + "guardCondition", "stop when the draft has no remaining issues", + "maxIterations", 4, + "blocks", java.util.List.of( + java.util.Map.of("blockId", "c1-b1", "blockType", "LLMBlock", + "purpose", "Revise the draft using feedback"))))))); + } + if (prompt.contains("TASK: BLOCK_CONFIG")) { + return TestAssistantResponses.wrap(java.util.Map.of( + "rationale", "The loop body revises the draft; its single open input receives feedback.", + "block", java.util.Map.of("blockId", "c1-b1", "name", "revise-draft", + "config", java.util.Map.of( + "prompt", "Revise this draft applying the latest feedback: ${{draft}}")))); + } + throw new IllegalStateException("Unexpected assistant prompt:\n" + prompt); + }; + Mockito.when(internalOllamaLLMProvider.generate(Mockito.eq(MODEL), Mockito.anyString())).thenAnswer(draftAnswer); + Mockito.when(internalOllamaLLMProvider.generateJson(Mockito.eq(MODEL), Mockito.anyString())).thenAnswer(draftAnswer); + + AssistantFlowResponse draftResponse = assistantController.draft( + new AssistantGenerationRequest("keep revising the draft until it passes review", MODEL, 1)); + assertTrue(draftResponse.valid(), () -> "Unexpected validation errors: " + draftResponse.validationErrors()); + + java.util.List capturedPrompts = new java.util.concurrent.CopyOnWriteArrayList<>(); + Answer fixAnswer = invocation -> { + String prompt = invocation.getArgument(1, String.class); + capturedPrompts.add(prompt); + if (prompt.contains("TASK: PLAN")) { + return TestAssistantResponses.wrap(java.util.Map.of( + "rationale", "Keep the loop as-is.", + "plan", java.util.Map.of( + "name", "Iterative revision", + "description", "Revise a draft until it has no issues.", + "blocks", java.util.List.of(), + "containers", java.util.List.of( + java.util.Map.of( + "containerId", "c1", + "containerType", "LoopContainer", + "purpose", "Revise until clean", + "operation", "KEEP", + "guardCondition", "stop when the draft has no remaining issues", + "maxIterations", 4, + "blocks", java.util.List.of( + java.util.Map.of("blockId", "c1-b1", "blockType", "LLMBlock", + "purpose", "Revise the draft using feedback", + "operation", "KEEP"))))))); + } + throw new IllegalStateException("Unexpected assistant prompt:\n" + prompt); + }; + Mockito.when(internalOllamaLLMProvider.generate(Mockito.eq(MODEL), Mockito.anyString())).thenAnswer(fixAnswer); + Mockito.when(internalOllamaLLMProvider.generateJson(Mockito.eq(MODEL), Mockito.anyString())).thenAnswer(fixAnswer); + + java.util.List errors = java.util.List.of(new ValidationError( + ValidationErrorCode.VALIDATION_ERROR, "flow", null, "connections", "some unrelated issue")); + assistantController.fix(new AssistantFixRequest("no-op fix", draftResponse.flow(), errors, MODEL, 0)); + + assertFalse(capturedPrompts.isEmpty(), "expected at least one prompt to have been sent during fix()"); + for (String prompt : capturedPrompts) { + assertFalse(prompt.contains("c1-expose-feedback"), + "guard subflow internals must not be shown to the model:\n" + prompt); + assertFalse(prompt.contains("c1-guard-evaluator"), + "guard subflow internals must not be shown to the model:\n" + prompt); + assertTrue(prompt.contains("backend-managed guard mechanism"), + "expected the redaction marker in place of the guard subflow:\n" + prompt); + } + } + @Test public void draftWiresMcpChainFromTopLevelProducerToConsumerInsideAContainer() { Answer answer = invocation -> {