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

fix: recover from empty-plan JSON and default missing required config fields 

Two robustness fixes found by testing the assistant live with the default
planning model (qwen3:14b), which produced a trivial single-block flow.

1. Empty-plan recovery. A reasoning model under Ollama's format=json can
   return a degenerate "{}" (it cannot emit its <think> block under the JSON
   grammar). invokeStructuredProvider accepted "{}" as a valid structured
   response, so the plan came back empty and validateAndNormalizePlan applied
   its single-LLMBlock fallback - masking the failure as a trivial flow. A
   degenerate JSON body ("{}", "[]", "") is now treated like a blank response
   and falls through to the non-json generate call, where such models emit the
   real content (parsed by the balanced-brace extractor). The single-block
   fallback stays as a genuine last resort. Fixes the trivial-flow symptom for
   reasoning planning models without a config change.

2. Missing required text field. When the model omits a required free-text
   config field (observed: MCPAgentChatBlockConfiguration.goalDescription),
   deserialization failed with a hard 502. buildBlock now fills any required,
   non-enum, non-structural string field the model left blank with a default
   derived from the block's purpose (ensureRequiredTextDefaults), so an
   omission degrades to a sensible default instead of a 502.

Tests: draftRecoversRealPlanWhenJsonModeReturnsEmptyObject (generateJson
returns "{}", generate returns the real 2-block plan -> flow has 2 blocks, not
the fallback); draftDefaultsMissingRequiredTextFieldInsteadOf502 (MCPAgentChat
with no goalDescription -> valid, goalDescription defaulted to the purpose).
Full assistant suite green; only the known Loop-container timing flake fails
under full-suite load (passes in isolation).
This commit is contained in:
Lucio Lelii 2026-08-03 11:05:58 +02:00
parent 9cc110464d
commit 653184eeae
2 changed files with 137 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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 <think> 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("''");
}
}

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

@ -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<String> 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<String> 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<String> 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";