diff --git a/src/main/java/it/cnr/isti/workflow/manager/assistant/FlowAssistantService.java b/src/main/java/it/cnr/isti/workflow/manager/assistant/FlowAssistantService.java index 18b7b8c..2dd9208 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/assistant/FlowAssistantService.java +++ b/src/main/java/it/cnr/isti/workflow/manager/assistant/FlowAssistantService.java @@ -1151,6 +1151,7 @@ public class FlowAssistantService { normalizedConfig.put("type", descriptor.configurationType()); normalizedConfig.put("name", defaultIfBlank(draft.name(), defaultIfBlank(blockPlan.purpose(), blockPlan.blockType()))); injectSystemManagedFields(normalizedConfig, descriptor, model); + ensureRequiredTextDefaults(normalizedConfig, descriptor, blockPlan); normalizeHttpServerCallAuthorization(normalizedConfig, blockPlan); normalizeMcpAgentSharedMemory(normalizedConfig, blockPlan, model, requireSharedMemorySemantics); ensureSequentialInputPlaceholder(normalizedConfig, descriptor, blockPlan, blockIndex, blockCount); @@ -1314,6 +1315,31 @@ public class FlowAssistantService { } } + /** + * Fills any required free-text configuration field the model omitted with a sensible default + * derived from the block's purpose, so a missing required string (e.g. MCPAgentChat's + * goalDescription) doesn't fail deserialization with a hard 502. Only plain required string + * fields are defaulted - enum fields (with allowed values) and structural fields are left + * untouched, as is any field the model already set. + */ + private void ensureRequiredTextDefaults(ObjectNode config, + BlockCatalogService.AssistantPromptBlockDescriptor descriptor, AssistantBlockPlan blockPlan) { + if (descriptor.configurationFields() == null) { + return; + } + for (BlockCatalogService.AssistantPromptFieldDescriptor field : descriptor.configurationFields()) { + if (!field.required() || field.structural() || !"string".equalsIgnoreCase(field.type())) { + continue; + } + if (field.allowedValues() != null && !field.allowedValues().isEmpty()) { + continue; + } + if (!hasTextValue(config.get(field.name()))) { + config.put(field.name(), defaultIfBlank(blockPlan.purpose(), "Assist the user with this step.")); + } + } + } + private void removeSystemManagedFields(ObjectNode config) { config.remove(List.of("provider", "model", "llmDescriptor", "ids", "inputs", "outputs", "skills")); } @@ -2759,7 +2785,12 @@ public class FlowAssistantService { if (structuredResponse != null) { logAssistantRawResponse(taskName, model, "json", attempt, maxAttempts, structuredResponse); } - if (structuredResponse != null && !structuredResponse.isBlank()) { + // A degenerate JSON body ("{}", "[]", "") is what a reasoning model (e.g. qwen3) emits + // under Ollama's format=json when it cannot output its block: valid JSON but no + // content. Treat it like a blank response and fall through to the non-json generate call + // below, where such models produce the real content (which the extractor then parses). + if (structuredResponse != null && !structuredResponse.isBlank() + && !isDegenerateJson(structuredResponse)) { return structuredResponse; } } catch (IllegalArgumentException e) { @@ -2855,4 +2886,17 @@ public class FlowAssistantService { || trimmed.startsWith("[") || trimmed.startsWith("```"); } + + /** + * True when a JSON response is structurally valid but carries no content - an empty object, + * empty array, or empty string. Such a body is treated as "no answer" (see + * invokeStructuredProvider). + */ + private boolean isDegenerateJson(String response) { + if (response == null) { + return true; + } + String trimmed = response.trim(); + return trimmed.equals("{}") || trimmed.equals("[]") || trimmed.equals("\"\"") || trimmed.equals("''"); + } } 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 ebf9c2b..7a7f8e9 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 @@ -2667,6 +2667,98 @@ public class AssistantControllerTest { assertEquals("draft", configuration.getFeedbackInput()); } + @Test + public void draftRecoversRealPlanWhenJsonModeReturnsEmptyObject() { + // A reasoning model under format=json can return a degenerate "{}" for the plan. That must + // not be accepted (it would fall back to a single trivial block); the backend retries via + // the non-json generate call, where such models emit the real plan. + Answer jsonAnswer = invocation -> { + String prompt = invocation.getArgument(1, String.class); + if (prompt.contains("TASK: PLAN")) { + return "{}"; // degenerate structured response + } + if (prompt.contains("TASK: BLOCK_CONFIG") + && prompt.contains("Current block to configure:\n{\n \"blockId\" : \"b1\"")) { + return TestAssistantResponses.wrap(java.util.Map.of("rationale", "first step", + "block", java.util.Map.of("blockId", "b1", "name", "gather", + "config", java.util.Map.of("prompt", "Gather requirements: ${{topic}}")))); + } + if (prompt.contains("TASK: BLOCK_CONFIG") + && prompt.contains("Current block to configure:\n{\n \"blockId\" : \"b2\"")) { + return TestAssistantResponses.wrap(java.util.Map.of("rationale", "second step", + "block", java.util.Map.of("blockId", "b2", "name", "design", + "config", java.util.Map.of("prompt", "Produce a design from: ${{input}}")))); + } + if (prompt.contains("TASK: CONNECTIONS")) { + return TestAssistantResponses.wrap(java.util.Map.of("rationale", "wire steps", + "connections", java.util.List.of(java.util.Map.of("fromBlockId", "b1", + "fromOutput", "response", "toBlockId", "b2", "toInput", "input")))); + } + throw new IllegalStateException("Unexpected assistant JSON prompt:\n" + prompt); + }; + // The non-json generate call returns the real multi-block plan for the PLAN task. + Answer textAnswer = invocation -> { + String prompt = invocation.getArgument(1, String.class); + if (prompt.contains("TASK: PLAN")) { + return TestAssistantResponses.wrap(java.util.Map.of( + "rationale", "Two sequential steps.", + "plan", java.util.Map.of("name", "Two step flow", "description", "desc", + "blocks", java.util.List.of( + java.util.Map.of("blockId", "b1", "blockType", "LLMBlock", "purpose", "Gather"), + java.util.Map.of("blockId", "b2", "blockType", "LLMBlock", "purpose", "Design"))))); + } + throw new IllegalStateException("Unexpected assistant text prompt:\n" + prompt); + }; + Mockito.when(internalOllamaLLMProvider.generateJson(Mockito.eq(MODEL), Mockito.anyString())) + .thenAnswer(jsonAnswer); + Mockito.when(internalOllamaLLMProvider.generate(Mockito.eq(MODEL), Mockito.anyString())) + .thenAnswer(textAnswer); + + AssistantFlowResponse response = assistantController.draft( + new AssistantGenerationRequest("build a two step pipeline", MODEL, 1)); + + assertNotNull(response); + assertTrue(response.valid(), () -> "Unexpected validation errors: " + response.validationErrors()); + // The real 2-block plan was recovered - NOT the single-block empty-plan fallback. + assertEquals(2, response.flow().flow().getBlocks().size()); + } + + @Test + public void draftDefaultsMissingRequiredTextFieldInsteadOf502() { + // MCPAgentChatBlockConfiguration requires goalDescription; if the model omits it, the + // backend fills it from the block purpose rather than failing deserialization with a 502. + Answer answer = invocation -> { + String prompt = invocation.getArgument(1, String.class); + if (prompt.contains("TASK: PLAN")) { + return TestAssistantResponses.wrap(java.util.Map.of("rationale", "One chat agent.", + "plan", java.util.Map.of("name", "Chat flow", "description", "desc", + "blocks", java.util.List.of(java.util.Map.of("blockId", "b1", + "blockType", "MCPAgentChat", "purpose", "Help the user plan a trip"))))); + } + if (prompt.contains("TASK: BLOCK_CONFIG")) { + // Note: no goalDescription supplied by the model. + return TestAssistantResponses.wrap(java.util.Map.of("rationale", "chat agent config", + "block", java.util.Map.of("blockId", "b1", "name", "trip-chat", + "config", java.util.Map.of("exposeHistory", true)))); + } + throw new IllegalStateException("Unexpected assistant prompt:\n" + prompt); + }; + Mockito.when(internalOllamaLLMProvider.generate(Mockito.eq(MODEL), Mockito.anyString())).thenAnswer(answer); + Mockito.when(internalOllamaLLMProvider.generateJson(Mockito.eq(MODEL), Mockito.anyString())).thenAnswer(answer); + + AssistantFlowResponse response = assistantController.draft( + new AssistantGenerationRequest("a chat assistant that helps plan a trip", MODEL, 1)); + + assertNotNull(response); + assertTrue(response.valid(), () -> "Unexpected validation errors: " + response.validationErrors()); + assertEquals(1, response.flow().flow().getBlocks().size()); + it.cnr.isti.workflow.manager.blocks.configurations.MCPAgentChatBlockConfiguration configuration = + (it.cnr.isti.workflow.manager.blocks.configurations.MCPAgentChatBlockConfiguration) + response.flow().flow().getBlocks().getFirst().getSpecificConfiguration(); + // The omitted required field was defaulted from the block purpose. + assertEquals("Help the user plan a trip", configuration.getGoalDescription()); + } + static class TestAssistantResponses { private static final String PROVIDER = "InternalOllama";