From e823d8d3573d48c6d0eacffff380929be8e6a1c4 Mon Sep 17 00:00:00 2001 From: Lucio Lelii Date: Mon, 3 Aug 2026 16:01:59 +0200 Subject: [PATCH] fix(assistant): statically sanitize brace/wrapper noise in connection endpoint names Answers "could the extra-braces problems be fixed statically?" - yes, the syntactic-noise class can and now is. The model sometimes mangles a connection endpoint name with purely syntactic noise: a stray/unbalanced brace ("{category"), an accidental ${{...}} wrapper, or a block-qualified reference ("classify.response"). normalizeBlockReference only did trim()+toLowerCase(), so "{category" never matched the real "category" input and the connection was silently dropped, leaving the flow disconnected. Added stripHandleNoise() - removes ${{ }} / {{ }} wrappers, stray braces/$/quotes, and a leading block-name qualifier (keeps the last dotted segment) - and wired it as a FALLBACK in findIoByName and resolveConnectionBlock: it only runs after the exact-name match already failed, so it can never change a currently-resolving reference, only rescue one that would otherwise be dropped. It never invents a name, so a genuinely-wrong reference (not just mangled) still fails and is dropped, as before. Scope note: this fixes the SYNTACTIC class only. Semantic/structural problems (duplicated logic, connections to non-existent handles, topology/deadlocks) are unaffected - those need the prompt-side and/or soft-vs-hard-validation work, not name cleanup. Test isolates the sanitization path by giving the target two inputs so the pre-existing single-input shortcut cannot mask it; verified by mutation that disabling the fallback drops the connection. 442/442. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../assistant/FlowAssistantService.java | 47 ++++++++++++++- .../controllers/AssistantControllerTest.java | 60 +++++++++++++++++++ 2 files changed, 105 insertions(+), 2 deletions(-) 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 973add6..6417b58 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 @@ -2019,7 +2019,14 @@ public class FlowAssistantService { if (direct != null) { return direct; } - return nodesByAlias.get(normalizeBlockReference(rawReference)); + FlowNode byAlias = nodesByAlias.get(normalizeBlockReference(rawReference)); + if (byAlias != null) { + return byAlias; + } + // Same syntactic-noise fallback as findIoByName: a brace-mangled or ${{...}}-wrapped block + // reference should still resolve to its node instead of dropping the whole connection. + String sanitized = normalizeBlockReference(stripHandleNoise(rawReference)); + return sanitized == null ? null : nodesByAlias.get(sanitized); } private FlowNode inferBlockByIo(String ioName, Collection nodes, boolean output) { @@ -2081,10 +2088,46 @@ public class FlowAssistantService { if (normalized == null) { return null; } - return descriptors.stream() + IODescriptor exact = descriptors.stream() .filter(io -> normalized.equals(normalizeBlockReference(io.getName()))) .findFirst() .orElse(null); + if (exact != null) { + return exact; + } + // Fallback for syntactic noise the model sometimes emits in a handle name - a stray or + // unbalanced brace ("{userReview"), an accidental ${{...}} wrapper ("${{userReview}}"), or a + // block-qualified reference ("SomeBlock.response"). Strip that noise and retry; this only + // runs after the exact match already failed, so it can never change a currently-resolving + // connection - it only rescues one that would otherwise be silently dropped. + String sanitized = normalizeBlockReference(stripHandleNoise(requestedName)); + if (sanitized == null || sanitized.equals(normalized)) { + return null; + } + return descriptors.stream() + .filter(io -> sanitized.equals(normalizeBlockReference(io.getName()))) + .findFirst() + .orElse(null); + } + + /** + * Removes purely-syntactic noise from a handle/block reference the model produced: placeholder + * wrappers ({@code ${{...}}} / {@code {{...}}}), stray braces, dollar signs and quotes, and a + * leading block-name qualifier (keeps the last dotted segment, so {@code "Review.response"} + * becomes {@code "response"}). Deterministic clean-up only - it never invents a name, so a + * reference that was genuinely wrong (not just mangled) still fails to resolve and is dropped. + */ + private String stripHandleNoise(String raw) { + if (raw == null) { + return null; + } + String cleaned = raw.replace("${{", "").replace("{{", "").replace("}}", ""); + cleaned = cleaned.replaceAll("[{}$\"']", "").trim(); + int lastDot = cleaned.lastIndexOf('.'); + if (lastDot >= 0 && lastDot < cleaned.length() - 1) { + cleaned = cleaned.substring(lastDot + 1).trim(); + } + return cleaned.isBlank() ? null : cleaned; } private void registerNodeAlias(Map nodesByAlias, String reference, FlowNode node) { 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 7ab130d..c926106 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 @@ -3189,6 +3189,66 @@ public class AssistantControllerTest { assertEquals("LoopContainer", response.flow().flow().getContainers().getFirst().getType().getName()); } + @Test + public void draftResolvesConnectionEndpointsDespiteBraceAndWrapperNoise() { + // The model sometimes mangles a connection endpoint name with purely-syntactic noise: a + // stray brace ("{category"), an accidental ${{...}} wrapper, or a block-qualified reference + // ("classify.response"). These used to fail the exact-name match and get silently dropped, + // leaving the flow disconnected. They must now be sanitized and still resolve to the real + // handle, so the connection survives. + // + // The target block deliberately has TWO inputs (category + detail): with more than one + // handle the "only one input, so use it" shortcut cannot fire, so the noisy "{category" + // name can ONLY be rescued by the sanitization path - this isolates the behaviour under test. + Answer answer = invocation -> { + String prompt = invocation.getArgument(1, String.class); + if (prompt.contains("TASK: PLAN")) { + return TestAssistantResponses.wrap(java.util.Map.of("rationale", "Two steps.", + "plan", java.util.Map.of("name", "Noise flow", "description", "desc", + "blocks", java.util.List.of( + java.util.Map.of("blockId", "b1", "blockType", "LLMBlock", "purpose", "Classify"), + java.util.Map.of("blockId", "b2", "blockType", "LLMBlock", "purpose", "Summarize"))))); + } + if (prompt.contains("TASK: BLOCK_CONFIG") + && prompt.contains("Current block to configure:\n{\n \"blockId\" : \"b1\"")) { + return TestAssistantResponses.wrap(java.util.Map.of("rationale", "classify", + "block", java.util.Map.of("blockId", "b1", "name", "classify", + "config", java.util.Map.of("prompt", "Classify: ${{ticket}}")))); + } + if (prompt.contains("TASK: BLOCK_CONFIG") + && prompt.contains("Current block to configure:\n{\n \"blockId\" : \"b2\"")) { + // Two placeholders -> two inputs (category, detail), so no single-input shortcut. + return TestAssistantResponses.wrap(java.util.Map.of("rationale", "summarize", + "block", java.util.Map.of("blockId", "b2", "name", "summarize", + "config", java.util.Map.of("prompt", + "Summarize the ${{category}} with this detail: ${{detail}}")))); + } + if (prompt.contains("TASK: CONNECTIONS")) { + // fromOutput is a block-qualified reference ("classify.response"); toInput carries a + // stray leading brace ("{category"). Both are pure noise around real handle names. + return TestAssistantResponses.wrap(java.util.Map.of("rationale", "wire with noisy names", + "connections", java.util.List.of(java.util.Map.of( + "fromBlockId", "b1", "fromOutput", "classify.response", + "toBlockId", "b2", "toInput", "{category")))); + } + 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("classify then summarize", MODEL, 1)); + + assertNotNull(response); + // The noisy connection survived: b1.response -> b2.category, resolved to the real handles. + java.util.List connections = response.flow().flow().getConnections(); + assertEquals(1, connections.size(), "the brace/wrapper-mangled connection must be rescued, not dropped"); + Connection connection = connections.getFirst(); + assertEquals("response", connection.getSourceName()); + assertEquals("category", connection.getTargetName(), + "the '{category' noise must resolve to the real 'category' input, not the other input"); + } + @Test public void draftDefaultsMissingRequiredTextFieldInsteadOf502() { // MCPAgentChatBlockConfiguration requires goalDescription; if the model omits it, the