diff --git a/src/main/java/it/cnr/isti/workflow/manager/blocks/configurations/LLMBlockConfiguration.java b/src/main/java/it/cnr/isti/workflow/manager/blocks/configurations/LLMBlockConfiguration.java index 14f3c42..5902943 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/blocks/configurations/LLMBlockConfiguration.java +++ b/src/main/java/it/cnr/isti/workflow/manager/blocks/configurations/LLMBlockConfiguration.java @@ -9,6 +9,7 @@ import java.util.List; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; +import it.cnr.isti.workflow.manager.blocks.factories.LLMBlockFactory; import it.cnr.isti.workflow.manager.blocks.factories.TemplateInputs; import it.cnr.isti.workflow.manager.blocks.types.LLMBlockType; import it.cnr.isti.workflow.manager.configurations.annotations.LongText; @@ -79,19 +80,39 @@ public class LLMBlockConfiguration extends BlockConfiguration { @JsonProperty(required = false) List requiredSuccessfulMcpTools = List.of(); + /** + * Fields the answer is split into, each becoming a port of its own. + * + *

Empty leaves the node as it has always been: one {@code response} port carrying the whole + * answer. Declaring fields is for a node that hands different things to different places - the + * address of something it started and the report about it, say - which a single block of text + * cannot do without whoever is downstream picking it apart by guesswork. + */ + @Structural + @Valid + @Size(max = LLMOutputField.MAX_PER_BLOCK) + @UiUniqueItemsBy("name") + @UiOrder(80) + @UiLabel("Structured outputs") + @UiDescription("Split the answer into named fields, each becoming a port. Leave empty for a single response port carrying the whole answer.") + @JsonProperty(required = false) + List outputs = List.of(); + @Builder public LLMBlockConfiguration(@NonNull String name, @JsonProperty(value = "llmDescriptor", required = false) LLMDescriptor llmDescriptor, String prompt, List skills, List mcpServers, - List requiredSuccessfulMcpTools) { + List requiredSuccessfulMcpTools, + List outputs) { super(name); this.llmDescriptor = llmDescriptor; this.prompt = prompt; this.skills = skills == null ? List.of() : List.copyOf(skills); this.mcpServers = mcpServers == null ? List.of() : List.copyOf(mcpServers); this.requiredSuccessfulMcpTools = requiredSuccessfulMcpTools == null ? List.of() : List.copyOf(requiredSuccessfulMcpTools); + this.outputs = outputs == null ? List.of() : List.copyOf(outputs); } @Override @@ -105,6 +126,7 @@ public class LLMBlockConfiguration extends BlockConfiguration { configuration.skills = List.of(); configuration.mcpServers = List.of(); configuration.requiredSuccessfulMcpTools = List.of(); + configuration.outputs = List.of(); return configuration; } @@ -162,4 +184,21 @@ public class LLMBlockConfiguration extends BlockConfiguration { && requiredSuccessfulMcpTools.stream().distinct().count() == requiredSuccessfulMcpTools.size(); } + @AssertTrue(message = "structured output names must be distinct, and none may be called 'response'") + @JsonIgnore + boolean areOutputNamesUsable() { + if (outputs == null || outputs.isEmpty()) { + return true; + } + List names = outputs.stream() + .map(LLMOutputField::name) + .filter(name -> name != null && !name.isBlank()) + .toList(); + // "response" is the port every LLM node already has, carrying the whole answer. A declared + // field of that name would produce two ports with one name, and the editor would let the + // flow be wired to whichever it happened to find first. + return names.stream().distinct().count() == names.size() + && names.stream().noneMatch(name -> name.equalsIgnoreCase(LLMBlockFactory.OUTPUT_NAME)); + } + } diff --git a/src/main/java/it/cnr/isti/workflow/manager/blocks/configurations/LLMOutputField.java b/src/main/java/it/cnr/isti/workflow/manager/blocks/configurations/LLMOutputField.java new file mode 100644 index 0000000..e2bcdeb --- /dev/null +++ b/src/main/java/it/cnr/isti/workflow/manager/blocks/configurations/LLMOutputField.java @@ -0,0 +1,52 @@ +// SPDX-FileCopyrightText: 2025-2026 Lucio Lelii - ISTI-CNR +// SPDX-License-Identifier: AGPL-3.0-or-later +// Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM. + +package it.cnr.isti.workflow.manager.blocks.configurations; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; + +import it.cnr.isti.workflow.manager.configurations.annotations.UiDescription; +import it.cnr.isti.workflow.manager.configurations.annotations.UiLabel; +import it.cnr.isti.workflow.manager.configurations.annotations.UiOrder; +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Pattern; +import jakarta.validation.constraints.Size; + +/** + * One field an LLM node produces as a port of its own, beside the whole answer. + * + *

A node that has to hand two different things to two different places - an address to open and + * a report to read, say - cannot do it through a single block of text without somebody downstream + * picking it apart by guesswork. Declaring the fields here is what turns the answer into ports the + * editor can wire, and what lets the node fail loudly when the model does not produce one. + * + *

The description is not documentation: it is given to the model as the definition of what + * belongs in the field, so it is the only instruction the model gets about it. + */ +@JsonIgnoreProperties(ignoreUnknown = true) +public record LLMOutputField( + + @NotBlank + @Size(max = 64) + @Pattern(regexp = "^[A-Za-z][A-Za-z0-9_-]*$", + message = "an output name must start with a letter and contain only letters, digits, '-' or '_'") + @JsonProperty(required = true) + @UiLabel("Name") + @UiDescription("Name of the port this field becomes. Must be unique within the node.") + @UiOrder(10) + String name, + + @NotBlank + @Size(max = 1000) + @JsonProperty(required = true) + @UiLabel("Description") + @UiDescription("What belongs in this field. Written for the model, which receives it as the " + + "field's definition and nothing else - so say what the value is, not why it matters.") + @UiOrder(20) + String description) { + + /** Enough for a node that hands several things onward, few enough to keep one prompt readable. */ + public static final int MAX_PER_BLOCK = 8; +} diff --git a/src/main/java/it/cnr/isti/workflow/manager/blocks/factories/LLMBlockFactory.java b/src/main/java/it/cnr/isti/workflow/manager/blocks/factories/LLMBlockFactory.java index e639105..57c49a4 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/blocks/factories/LLMBlockFactory.java +++ b/src/main/java/it/cnr/isti/workflow/manager/blocks/factories/LLMBlockFactory.java @@ -4,6 +4,7 @@ package it.cnr.isti.workflow.manager.blocks.factories; +import java.util.ArrayList; import java.util.List; import org.springframework.beans.factory.annotation.Autowired; @@ -12,6 +13,7 @@ import it.cnr.isti.workflow.manager.blocks.IOCapability; import it.cnr.isti.workflow.manager.blocks.IOCapabilityType; import it.cnr.isti.workflow.manager.blocks.Block; import it.cnr.isti.workflow.manager.blocks.configurations.LLMBlockConfiguration; +import it.cnr.isti.workflow.manager.blocks.configurations.LLMOutputField; import it.cnr.isti.workflow.manager.blocks.types.LLMBlockType; import it.cnr.isti.workflow.manager.ios.IODescriptor; import it.cnr.isti.workflow.manager.ios.IOType; @@ -35,6 +37,15 @@ public class LLMBlockFactory implements BlockFactory declaredOutputs(LLMBlockConfiguration configuration) { + List outputs = new ArrayList<>(); + outputs.add(IODescriptor.output(OUTPUT_NAME, IOType.TEXT, false, OUTPUT_CAPABILITIES)); + for (LLMOutputField field : configuration.getOutputs()) { + outputs.add(IODescriptor.output(field.name(), IOType.TEXT, false, OUTPUT_CAPABILITIES)); + } + return outputs; + } + private List retrieveInputs(String input) { return TemplateInputs.toInputs(TemplateInputs.extractNames(input), IOType.TEXT, INPUT_CAPABILITIES); } @@ -54,7 +65,10 @@ public class LLMBlockFactory implements BlockFactory block = Block.builder() .inputs(inputs) - .output(IODescriptor.output(OUTPUT_NAME, IOType.TEXT, false, OUTPUT_CAPABILITIES)) + // response always exists and carries the whole answer, declared fields or not: a + // node whose structured output disappoints is still worth reading in full, and a + // flow already wired to it must not break when fields are added. + .outputs(declaredOutputs(configuration)) .specificConfiguration(configuration) .type(blockType) .build(); diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/LLMExecutor.java b/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/LLMExecutor.java index 6d0dfb4..99285e2 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/LLMExecutor.java +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/LLMExecutor.java @@ -69,6 +69,11 @@ public class LLMExecutor implements BlockExecutor { prompt = skillsInstructions + System.lineSeparator() + System.lineSeparator() + prompt; } prompt = BiasRuntimeSupport.decoratePrompt(prompt, executionVariables); + // Last, so it is the closest instruction to where the model starts writing - and after the + // bias decoration, which must not come between the ask and the answer. + if (!config.getOutputs().isEmpty()) { + prompt = prompt + StructuredLLMOutputs.instructionFor(config.getOutputs()); + } LLMDescriptor llmDescriptor = LLMDescriptorInputBinding.resolve(config.getLlmDescriptor(), inputs, executionVariables); LLMProvider llmProvider = llmProviders.get(llmDescriptor.provider()); @@ -91,7 +96,7 @@ public class LLMExecutor implements BlockExecutor { if (config.getMcpServers() != null && !config.getMcpServers().isEmpty()) { String response = llmToolLoop.run(llmProvider, llmDescriptor, credential, prompt, config.getMcpServers(), executionVariables, eventLogger, config.getRequiredSuccessfulMcpTools()); - return Map.of(LLMBlockFactory.OUTPUT_NAME, response); + return StructuredLLMOutputs.split(response, config.getOutputs(), LLMBlockFactory.OUTPUT_NAME); } String response = llmProvider.generate(llmDescriptor.model(), prompt, credential, @@ -102,7 +107,7 @@ public class LLMExecutor implements BlockExecutor { Map.of("provider", llmDescriptor.provider(), "model", llmDescriptor.model())); } - return Map.of(LLMBlockFactory.OUTPUT_NAME, response); + return StructuredLLMOutputs.split(response, config.getOutputs(), LLMBlockFactory.OUTPUT_NAME); } @Override diff --git a/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/StructuredLLMOutputs.java b/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/StructuredLLMOutputs.java new file mode 100644 index 0000000..f7d10a2 --- /dev/null +++ b/src/main/java/it/cnr/isti/workflow/manager/executions/executors/blocks/StructuredLLMOutputs.java @@ -0,0 +1,150 @@ +// SPDX-FileCopyrightText: 2025-2026 Lucio Lelii - ISTI-CNR +// SPDX-License-Identifier: AGPL-3.0-or-later +// Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM. + +package it.cnr.isti.workflow.manager.executions.executors.blocks; + +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import it.cnr.isti.workflow.manager.blocks.configurations.LLMOutputField; +import tools.jackson.core.JacksonException; +import tools.jackson.databind.JsonNode; +import tools.jackson.databind.ObjectMapper; + +/** + * Turns one answer into the several ports a node declared, by asking for them and then reading them. + * + *

The model is told to close its reply with a JSON object, and that object is read back out of + * the text. Asking is the whole mechanism: there is no provider feature behind this, so it works + * the same whether the node made one call or ran a whole tool loop, and with any provider the + * catalogue offers. + * + *

What it will not do is guess. A field the model did not produce is a failure, named and + * refused - the same choice the tool loop already makes for a tool call typed as prose. A flow + * wired to a port that silently arrives empty is worse than one that stops: everything downstream + * treats the emptiness as an answer. + */ +final class StructuredLLMOutputs { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + private StructuredLLMOutputs() { + } + + /** + * The instruction appended to the prompt. Deliberately at the end and deliberately explicit + * about "nothing after it": a model that keeps talking past the object makes the closing brace + * ambiguous to find. + */ + static String instructionFor(List fields) { + StringBuilder instruction = new StringBuilder(); + instruction.append(System.lineSeparator()).append(System.lineSeparator()) + .append("When you have finished, end your reply with a single JSON object and nothing after it.") + .append(" It must have exactly these keys, each a string:") + .append(System.lineSeparator()); + for (LLMOutputField field : fields) { + instruction.append("- \"").append(field.name()).append("\": ").append(field.description()) + .append(System.lineSeparator()); + } + instruction.append("Write the object as the last thing in your reply. Everything you want to say") + .append(" other than those values goes before it."); + return instruction.toString(); + } + + /** + * Reads the declared fields out of the answer. + * + * @return the response itself plus one entry per declared field, ready to become ports. + */ + static Map split(String response, List fields, String responsePortName) { + Map result = new LinkedHashMap<>(); + result.put(responsePortName, response); + if (fields == null || fields.isEmpty()) { + return result; + } + + JsonNode object = lastJsonObjectIn(response); + if (object == null) { + throw new IllegalStateException("The model was asked to end its reply with a JSON object holding " + + names(fields) + ", and did not. Its answer is on the response port, unchanged."); + } + for (LLMOutputField field : fields) { + JsonNode value = object.get(field.name()); + if (value == null || value.isNull()) { + throw new IllegalStateException("The model's closing JSON object has no \"" + field.name() + + "\". It was asked for " + names(fields) + "."); + } + // A model asked for a string sometimes answers with a number or a nested object. Its + // text form is what a port carries, so take that rather than refusing over a type. + result.put(field.name(), value.isString() ? value.asString() : value.toString()); + } + return result; + } + + /** + * The last balanced {...} in the text, parsed. + * + *

The last rather than the first: a reply that discusses JSON before producing its own - an + * agent reporting on a file it wrote, say - would otherwise hand back the example instead of + * the answer. + * + *

Scanned forwards, tracking depth and whether the scan is inside a string. Forwards + * matters: going backwards, a quote cannot be told from an escaped one without counting the + * backslashes before it, and getting that wrong would end an object early at a brace that was + * only ever part of a value. + */ + private static JsonNode lastJsonObjectIn(String text) { + if (text == null) { + return null; + } + JsonNode last = null; + int depth = 0; + int objectStart = -1; + boolean inString = false; + boolean escaped = false; + for (int index = 0; index < text.length(); index++) { + char character = text.charAt(index); + if (inString) { + if (escaped) { + escaped = false; + } else if (character == '\\') { + escaped = true; + } else if (character == '"') { + inString = false; + } + continue; + } + if (character == '"') { + inString = true; + } else if (character == '{') { + if (depth == 0) { + objectStart = index; + } + depth++; + } else if (character == '}' && depth > 0) { + depth--; + if (depth == 0 && objectStart >= 0) { + JsonNode parsed = parseOrNull(text.substring(objectStart, index + 1)); + if (parsed != null && parsed.isObject()) { + last = parsed; + } + } + } + } + return last; + } + + private static JsonNode parseOrNull(String candidate) { + try { + return MAPPER.readTree(candidate); + } catch (JacksonException notJson) { + return null; + } + } + + private static String names(List fields) { + return fields.stream().map(LLMOutputField::name).reduce((a, b) -> a + ", " + b).orElse(""); + } +} diff --git a/src/test/java/it/cnr/isti/workflow/manager/executions/executors/blocks/StructuredLLMOutputsTest.java b/src/test/java/it/cnr/isti/workflow/manager/executions/executors/blocks/StructuredLLMOutputsTest.java new file mode 100644 index 0000000..661e4db --- /dev/null +++ b/src/test/java/it/cnr/isti/workflow/manager/executions/executors/blocks/StructuredLLMOutputsTest.java @@ -0,0 +1,130 @@ +// SPDX-FileCopyrightText: 2025-2026 Lucio Lelii - ISTI-CNR +// SPDX-License-Identifier: AGPL-3.0-or-later +// Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM. + +package it.cnr.isti.workflow.manager.executions.executors.blocks; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.Test; + +import it.cnr.isti.workflow.manager.blocks.configurations.LLMOutputField; + +/** Reading declared fields out of an answer a model wrote, including the ways it writes them badly. */ +class StructuredLLMOutputsTest { + + private static final List FIELDS = List.of( + new LLMOutputField("previewUrl", "the address the preview is reachable at"), + new LLMOutputField("report", "what the browser showed")); + + @Test + void splitsTheClosingObjectIntoPortsAndKeepsTheWholeAnswer() { + String answer = "I built it and looked at it.\n" + + "{\"previewUrl\": \"https://host/preview/exec-a/\", \"report\": \"the page said Ciao\"}"; + + Map result = StructuredLLMOutputs.split(answer, FIELDS, "response"); + + assertEquals("https://host/preview/exec-a/", result.get("previewUrl")); + assertEquals("the page said Ciao", result.get("report")); + assertEquals(answer, result.get("response"), "the whole answer stays readable on its own port"); + } + + @Test + void takesTheClosingObjectRatherThanOneTheAnswerMerelyTalksAbout() { + // An agent that has just written a JSON file tends to quote it. The first object in the + // reply is that file; the last one is the answer being asked for. + String answer = "I wrote {\"schema_version\": 1, \"items\": []} to state.json, then checked it.\n" + + "{\"previewUrl\": \"https://host/p/\", \"report\": \"fine\"}"; + + Map result = StructuredLLMOutputs.split(answer, FIELDS, "response"); + + assertEquals("https://host/p/", result.get("previewUrl")); + } + + @Test + void readsAnObjectWhoseValuesContainBraces() { + // Counting braces without knowing where strings are would end this object early, at the + // brace inside the value, and leave a fragment that does not parse. + String answer = "done\n{\"previewUrl\": \"https://host/p/\", \"report\": \"it printed {ok} twice\"}"; + + Map result = StructuredLLMOutputs.split(answer, FIELDS, "response"); + + assertEquals("it printed {ok} twice", result.get("report")); + } + + @Test + void readsAnObjectWithNestedObjectsInside() { + String answer = "{\"previewUrl\": \"u\", \"report\": \"r\", \"extra\": {\"nested\": {\"deep\": 1}}}"; + + Map result = StructuredLLMOutputs.split(answer, FIELDS, "response"); + + assertEquals("u", result.get("previewUrl")); + assertEquals("r", result.get("report")); + } + + @Test + void readsAnObjectWhoseStringHoldsAnEscapedQuoteBeforeABrace() { + // The escape has to be understood, or the scan thinks the string ended at the inner quote + // and starts treating the rest of the value as structure. + String answer = "{\"previewUrl\": \"u\", \"report\": \"it said \\\"done\\\" and {stopped}\"}"; + + Map result = StructuredLLMOutputs.split(answer, FIELDS, "response"); + + assertEquals("it said \"done\" and {stopped}", result.get("report")); + } + + @Test + void refusesAnAnswerWithNoObjectAtAll() { + // The alternative is handing the flow empty ports, which everything downstream would treat + // as an answer rather than as an absence. + IllegalStateException failure = assertThrows(IllegalStateException.class, + () -> StructuredLLMOutputs.split("I could not manage it.", FIELDS, "response")); + + assertTrue(failure.getMessage().contains("previewUrl, report"), failure.getMessage()); + } + + @Test + void namesTheFieldTheModelLeftOut() { + IllegalStateException failure = assertThrows(IllegalStateException.class, + () -> StructuredLLMOutputs.split("{\"previewUrl\": \"u\"}", FIELDS, "response")); + + assertTrue(failure.getMessage().contains("\"report\""), failure.getMessage()); + } + + @Test + void refusesANullFieldRatherThanPassingItOnAsEmpty() { + assertThrows(IllegalStateException.class, + () -> StructuredLLMOutputs.split("{\"previewUrl\": \"u\", \"report\": null}", FIELDS, "response")); + } + + @Test + void carriesANonStringValueAsItsText() { + // Worth taking rather than refusing: the port carries text either way, and a model that + // answered 8080 where a string was asked for has still answered. + Map result = StructuredLLMOutputs.split( + "{\"previewUrl\": \"u\", \"report\": 8080}", FIELDS, "response"); + + assertEquals("8080", result.get("report")); + } + + @Test + void leavesAnswersAloneWhenNoFieldsAreDeclared() { + Map result = StructuredLLMOutputs.split("just prose", List.of(), "response"); + + assertEquals(Map.of("response", "just prose"), result); + } + + @Test + void theInstructionNamesEveryFieldAndItsDescription() { + String instruction = StructuredLLMOutputs.instructionFor(FIELDS); + + assertTrue(instruction.contains("\"previewUrl\""), instruction); + assertTrue(instruction.contains("the address the preview is reachable at"), instruction); + assertTrue(instruction.contains("\"report\""), instruction); + } +}