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 index f7d10a2..7ff5e7b 100644 --- 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 @@ -8,6 +8,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import it.cnr.isti.workflow.manager.executions.NodeExecutionException; import it.cnr.isti.workflow.manager.blocks.configurations.LLMOutputField; import tools.jackson.core.JacksonException; import tools.jackson.databind.JsonNode; @@ -30,9 +31,34 @@ final class StructuredLLMOutputs { private static final ObjectMapper MAPPER = new ObjectMapper(); + /** The step's error code when the answer does not carry the fields the node declared. */ + static final String OUTPUT_MISSING = "LLM_STRUCTURED_OUTPUT_MISSING"; + + /** + * How much of the model's own words a failure quotes: enough to see what it did instead, + * not so much that the error drowns the message it belongs to. + */ + private static final int QUOTED_CHARS = 600; + private StructuredLLMOutputs() { } + /** + * The end of the reply, where the object was asked to be - and so where a model that wrote + * something else instead shows what that was. + */ + private static String tail(String response) { + if (response == null || response.isBlank()) { + return "(nothing)"; + } + String text = response.strip(); + return text.length() <= QUOTED_CHARS ? text : "..." + text.substring(text.length() - QUOTED_CHARS); + } + + private static String head(String text) { + return text.length() <= QUOTED_CHARS ? text : text.substring(0, QUOTED_CHARS) + "..."; + } + /** * 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 @@ -67,14 +93,15 @@ final class StructuredLLMOutputs { 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."); + throw new NodeExecutionException(OUTPUT_MISSING, "The model was asked to end its reply with a JSON" + + " object holding " + names(fields) + ", and did not. Its reply ended: " + tail(response)); } 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) + "."); + throw new NodeExecutionException(OUTPUT_MISSING, "The model's closing JSON object has no \"" + + field.name() + "\". It was asked for " + names(fields) + ". The object it wrote: " + + head(object.toString())); } // 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. 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 index 661e4db..66ffc69 100644 --- 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 @@ -13,6 +13,7 @@ import java.util.Map; import org.junit.jupiter.api.Test; +import it.cnr.isti.workflow.manager.executions.NodeExecutionException; 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. */ @@ -82,23 +83,38 @@ class StructuredLLMOutputsTest { 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, + NodeExecutionException failure = assertThrows(NodeExecutionException.class, () -> StructuredLLMOutputs.split("I could not manage it.", FIELDS, "response")); + assertEquals(StructuredLLMOutputs.OUTPUT_MISSING, failure.getErrorCode()); assertTrue(failure.getMessage().contains("previewUrl, report"), failure.getMessage()); + // The failed step publishes no ports, so the reply is only ever seen here. + assertTrue(failure.getMessage().endsWith("Its reply ended: I could not manage it."), failure.getMessage()); + } + + @Test + void quotesOnlyTheEndOfALongReplyWhereTheObjectShouldHaveBeen() { + String longReply = "x".repeat(5_000) + " and so the page is ready."; + + NodeExecutionException failure = assertThrows(NodeExecutionException.class, + () -> StructuredLLMOutputs.split(longReply, FIELDS, "response")); + + assertTrue(failure.getMessage().endsWith("and so the page is ready."), failure.getMessage()); + assertTrue(failure.getMessage().length() < 1_000, "quoted, not dumped: " + failure.getMessage().length()); } @Test void namesTheFieldTheModelLeftOut() { - IllegalStateException failure = assertThrows(IllegalStateException.class, + NodeExecutionException failure = assertThrows(NodeExecutionException.class, () -> StructuredLLMOutputs.split("{\"previewUrl\": \"u\"}", FIELDS, "response")); assertTrue(failure.getMessage().contains("\"report\""), failure.getMessage()); + assertTrue(failure.getMessage().endsWith("The object it wrote: {\"previewUrl\":\"u\"}"), failure.getMessage()); } @Test void refusesANullFieldRatherThanPassingItOnAsEmpty() { - assertThrows(IllegalStateException.class, + assertThrows(NodeExecutionException.class, () -> StructuredLLMOutputs.split("{\"previewUrl\": \"u\", \"report\": null}", FIELDS, "response")); }