This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch CAMEL-26960-answer-marks in repository https://gitbox.apache.org/repos/asf/camel.git
commit 3d34c81fcef952167d455692b6881639f38ce7ce Author: Claus Ibsen <[email protected]> AuthorDate: Sat Nov 14 09:39:09 2026 +0100 CAMEL-26960: camel-jbang - mark the mistakes in the code of AI answers, such as options that do not exist The AI panel and camel ask check the code blocks of an answer against the catalog, as the simple expressions of an answer already are (CAMEL-24805): the keys and options of YAML routes and of fragments of them, the options of endpoint uris, and the camel.* keys of properties lines in any block. A fragment is checked in what it belongs to: a from: without its route as an entry of a route file, steps and a single EIP as the steps of a route, a redeliveryPolicy: under an onException. Only what does not exist is marked, and a value of the wrong type when the validator has a hint for it (onException written as a list): code in an answer leaves things out on purpose. The panel shows a line under the block with each mistake, camel ask prints it on stderr. The answer is not sent back for a correction; the model is told with the next question, as it is of a simple expression it did not fix. Over the 318 answers of a benchmark run with a local model (qwen3.6:35b): 53 marks in 27 answers, each one a mistake. The run also found two bugs of the validator: an option commented out under parameters: was reported as the unknown option '# idempotentRepository', and a bean with its name: and an unknown key was told that the name is a property and not the key. Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Claus Ibsen <[email protected]> --- .../modules/ROOT/pages/camel-jbang-ai.adoc | 12 + .../modules/ROOT/pages/camel-jbang-tui-ai.adoc | 19 ++ .../apache/camel/dsl/jbang/core/commands/Ask.java | 12 +- .../dsl/jbang/core/commands/ai/AnswerChecks.java | 281 +++++++++++++++++++++ .../dsl/jbang/core/commands/ai/EndpointChecks.java | 4 +- .../dsl/jbang/core/commands/ai/YamlFragment.java | 145 +++++++++++ .../camel/dsl/jbang/core/commands/AskLoopTest.java | 14 + .../core/commands/ai/AnswerCodeChecksTest.java | 260 +++++++++++++++++++ .../commands/ai/SourceValidatorEndpointTest.java | 17 ++ .../camel/dsl/jbang/core/commands/tui/AiPanel.java | 10 +- .../dsl/jbang/core/commands/tui/AiPanelTest.java | 63 +++++ .../camel/dsl/yaml/validator/SchemaHints.java | 16 +- .../validator/YamlValidatorPropertyHintTest.java | 25 ++ 13 files changed, 872 insertions(+), 6 deletions(-) diff --git a/docs/user-manual/modules/ROOT/pages/camel-jbang-ai.adoc b/docs/user-manual/modules/ROOT/pages/camel-jbang-ai.adoc index 8cfb5d7eab93..d48f871fbafc 100644 --- a/docs/user-manual/modules/ROOT/pages/camel-jbang-ai.adoc +++ b/docs/user-manual/modules/ROOT/pages/camel-jbang-ai.adoc @@ -59,6 +59,18 @@ With a question, the answer is the only output on stdout, so a script can use it `--show-tools` and the token usage of `--show-stats` are printed on stderr. `camel ask` exits with `1` when the LLM cannot answer, such as when it is not reachable or the call fails. +The code in an answer is checked against the Camel catalog, as it is what you copy. An invalid simple expression gets +the model one turn to fix it. What does not exist in the code blocks is printed on stderr under the answer, one +`Check:` line each: a key or an option the YAML DSL does not have, an option the component of an endpoint does not +have, and a `camel.*` key that is not a Camel option. For example: + +[source,text] +---- +Check: code block 1, line 4: `deleteAfterRead` is not an option of the `file` component (did you mean `delete`?) +---- + +In a chat, the model is told of them with your next question, so its next answer does not repeat them. + === What asks first Most tools only read, and run without asking. A call that changes the project, diff --git a/docs/user-manual/modules/ROOT/pages/camel-jbang-tui-ai.adoc b/docs/user-manual/modules/ROOT/pages/camel-jbang-tui-ai.adoc index a0108b4ad1c1..050c843b71c2 100644 --- a/docs/user-manual/modules/ROOT/pages/camel-jbang-tui-ai.adoc +++ b/docs/user-manual/modules/ROOT/pages/camel-jbang-tui-ai.adoc @@ -183,6 +183,25 @@ does not fix it the answer is shown as it is with a *Simple check* line under it and the error, and the AI log has the exchange. Placeholders that are not simple (a Maven `$\{camel-version}`, a shell variable) are left alone. +The code blocks of an answer are checked for what does not exist: a key or an option the YAML DSL does +not have (`multiplier` in a `redeliveryPolicy`, where the option is `backOffMultiplier`), an option the +component of an endpoint does not have (`deleteAfterRead` on `file`), and a `camel.*` key that is not a +Camel option (`camel.server.context-path`), in any block. A snippet is checked as a part of what it +belongs to: a `from:` without its route, a list of steps and a single EIP as the steps of a route, and a +`redeliveryPolicy:` under an `onException`. Each mistake is marked with a line under its code block, +with its line in the block and the hint of the validator when it has one: + +[source,text] +---- +ā line 5: `deleteAfterRead` is not an option of the `file` component (did you mean `delete`?) +---- + +The answer is not sent back for a correction, which would cost a local model the whole answer again. +The model is told of the marks, and of a simple expression it did not fix, with your next question, so +its next answer does not repeat them. What a snippet leaves out on purpose is not marked: a required +option, the steps of a route, a placeholder value, the options of an EIP shown without the EIP. *Copy* +still copies only the code of a block. + Press *Ctrl+U* while the AI panel is open to toggle the AI Usage view. It shows token consumption from the embedded AI prompt (*TUI ask*) and from LLM calls made by the monitored integration (*integration*), the latter taken from OpenTelemetry GenAI spans when observability is enabled on the integration. The diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/Ask.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/Ask.java index d5f15e75042a..8bca3919aa8c 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/Ask.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/Ask.java @@ -125,6 +125,8 @@ public class Ask extends CamelCommand { String lastAnswer; // the model that answered, and its API, such as qwen3.6 (ollama) String usedModel; + // the mistakes left in the last answer of a chat, which the model is told with the next question (CAMEL-26960) + private String pendingNote; private long targetPid; private AskTools askTools; @@ -245,7 +247,9 @@ public class Ask extends CamelCommand { LlmClient client, String systemPrompt, List<LlmClient.ToolDef> tools, List<LlmClient.Message> messages, String userQuestion) { - messages.add(LlmClient.Message.user(userQuestion)); + String note = pendingNote; + pendingNote = null; + messages.add(LlmClient.Message.user(note == null ? userQuestion : "[" + note + "]\n" + userQuestion)); long startTime = System.currentTimeMillis(); LlmClient.TokenUsage totalUsage = LlmClient.TokenUsage.EMPTY; @@ -290,6 +294,12 @@ public class Ask extends CamelCommand { "(the answer has an invalid simple expression: asking for a corrected answer)"); continue; } + // the code of the answer is checked too: what does not exist, and a simple expression left invalid, + // is printed under the answer, and in a chat the model is told with the next question (CAMEL-26960) + List<AnswerChecks.Mark> marks = AnswerChecks.checkCode(response.text()); + marks.forEach(m -> printer().printDiagnostic("Check: " + m.describe())); + problems.forEach(p -> printer().printDiagnostic("Check: invalid simple expression " + p.message())); + pendingNote = AnswerChecks.historyNote(marks, problems); messages.add(LlmClient.Message.assistantWithToolCalls(response.text(), List.of())); lastAnswer = response.text(); printStats(totalUsage, startTime); diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/AnswerChecks.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/AnswerChecks.java index 441fcf280785..ab88cda9f200 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/AnswerChecks.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/AnswerChecks.java @@ -17,16 +17,22 @@ package org.apache.camel.dsl.jbang.core.commands.ai; import java.util.ArrayList; +import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import com.networknt.schema.Error; import org.apache.camel.catalog.CamelCatalog; import org.apache.camel.catalog.DefaultCamelCatalog; import org.apache.camel.catalog.LanguageValidationResult; +import org.apache.camel.dsl.yaml.validator.ProjectFacts; +import org.apache.camel.dsl.yaml.validator.YamlValidator; import org.apache.camel.tooling.model.LanguageModel; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,6 +47,11 @@ import org.slf4j.LoggerFactory; * date, ...), so a Maven {@code ${camel-version}} or a shell variable in the same answer is left alone. What the * catalog cannot judge is skipped as in {@link SimpleChecks}: a function of a language that is not on the classpath, a * property placeholder used as a logical operand. + * <p> + * The code blocks of an answer are checked for what does not exist (CAMEL-26960): the keys and options of YAML routes + * and of fragments of them ({@link YamlFragment}), the options of their endpoint uris, and the {@code camel.*} keys of + * properties lines. A model told to look options up and to validate its snippets wrote more of them and validated none, + * so the check does not wait to be asked. Its mistakes are marked under their block, not sent back for a correction. */ public final class AnswerChecks { @@ -53,6 +64,46 @@ public final class AnswerChecks { } } + /** + * A mistake in the code of an answer: a key, an option or a {@code camel.*} property that does not exist, which a + * user who copies the code takes into a route. block is the number of the fenced code block in the answer, 1 for + * the first; line is the line in that block, 1 for the line after the fence, or 0 when it is not known. + */ + public record Mark(int block, int line, String message) { + + /** {@code code block 2, line 4: `multiplier` is not an option of `redeliveryPolicy`}. */ + public String describe() { + return "code block " + block + (line > 0 ? ", line " + line : "") + ": " + message; + } + } + + /** A fenced code block of an answer: its number (1 for the first), language, code and the line of its end fence. */ + private record CodeBlock(int number, String language, String code, int closingLine) { + } + + /** A camel.* key at the start of a properties line: camel.server.port=8080. */ + private static final Pattern CAMEL_PROPERTY = Pattern.compile("^\\s*(camel\\.[\\w.\\[\\]-]+)\\s*="); + + /** The camel.* keys of the CLI and of other runtimes, which the catalog does not know. */ + private static final List<String> OTHER_PROPERTIES = List.of("camel.jbang.", "camel.springboot.", "camel.k."); + + /** What the properties check says of a key that does not exist, and the hint after it. */ + private static final Pattern UNKNOWN_PROPERTY = Pattern.compile("Unknown (option|component|dataformat|language)(.*)$"); + + /** What the endpoint check says of an option the component does not have: line, scheme, option, the hint after. */ + private static final Pattern UNKNOWN_ENDPOINT_OPTION + = Pattern.compile("^Line (\\d+): ([^:\\s]+): Unknown option '([^']+)'(.*)$"); + + /** + * A value of the wrong type, as the schema says it: what was found, and the hint of the validator if it has one. + */ + private static final Pattern TYPE_FOUND = Pattern.compile("^(\\w+) found, \\w+ expected(?: \\((.+)\\))?$"); + + /** The catalog's suggestions, and the words after them. */ + private static final Pattern DID_YOU_MEAN = Pattern.compile("^\\.?\\s*Did you mean: \\[([^]]*)]\\.?(.*)$"); + + private static final Pattern UNKNOWN_KEY = Pattern.compile("property '([^']+)'"); + private static final Logger LOG = LoggerFactory.getLogger(AnswerChecks.class); private static volatile CamelCatalog defaultCatalog; @@ -147,6 +198,236 @@ public final class AnswerChecks { return new ArrayList<>(problems.values()); } + /** The mistakes of the code blocks of an answer, checked against the default catalog. */ + public static List<Mark> checkCode(String markdown) { + return checkCode(markdown, defaultCatalog()); + } + + /** + * The mistakes of the code blocks of an answer, checked against the given catalog: the keys and options of YAML + * routes and of fragments of them, the options of their endpoint uris, and the camel.* keys of properties lines in + * any block. Code in an answer is partial on purpose, so only what does not exist is a mistake, not a missing + * option, a placeholder value or a fragment shown without its route: a mark that is wrong costs the user's trust in + * the others. + */ + public static List<Mark> checkCode(String markdown, CamelCatalog catalog) { + if (markdown == null || markdown.isBlank() || catalog == null) { + return List.of(); + } + List<Mark> marks = new ArrayList<>(); + for (CodeBlock block : codeBlocks(markdown)) { + try { + marks.addAll(propertyMarks(block, catalog)); + if (block.language().equals("yaml") || block.language().equals("yml")) { + marks.addAll(yamlMarks(block, catalog)); + } + } catch (Exception e) { + // best effort, as for the simple expressions: the cause is in the debug log + LOG.debug("Cannot check code block {} of the answer", block.number(), e); + } + } + return marks; + } + + /** + * The answer with its marks: under each code block with a mistake, one line per mark, indented as the fence is, so + * the user sees it where the code is. The code is left as it is, so copying a code block copies only the code. + */ + public static String withMarks(String markdown, List<Mark> marks) { + if (markdown == null || marks == null || marks.isEmpty()) { + return markdown; + } + Map<Integer, Integer> blockEndingAt = new HashMap<>(); + for (CodeBlock block : codeBlocks(markdown)) { + blockEndingAt.put(block.closingLine(), block.number()); + } + String[] lines = markdown.split("\n", -1); + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < lines.length; i++) { + sb.append(lines[i]); + Integer number = blockEndingAt.get(i); + List<Mark> here = number == null ? List.of() : marks.stream().filter(m -> m.block() == number).toList(); + if (!here.isEmpty()) { + String indent = lines[i].substring(0, lines[i].indexOf('`')); + sb.append('\n'); + for (Mark mark : here) { + sb.append('\n').append(indent).append("ā ").append(mark.line() > 0 ? "line " + mark.line() + ": " : "") + .append(mark.message()); + } + if (i + 1 < lines.length && !lines[i + 1].isBlank()) { + sb.append('\n'); + } + } + if (i < lines.length - 1) { + sb.append('\n'); + } + } + return sb.toString(); + } + + /** + * What the model is told with the next question about the mistakes left in its last answer, the marks of its code + * and the simple expressions it did not fix, so that its next answer does not repeat them; null when there are + * none. The user sees them under the answer already. + */ + public static String historyNote(List<Mark> marks, List<Problem> problems) { + List<String> found = new ArrayList<>(); + if (marks != null) { + marks.forEach(m -> found.add(m.describe())); + } + if (problems != null) { + problems.forEach(p -> found.add("invalid simple expression " + p.message())); + } + return found.isEmpty() + ? null + : "Checked against the Camel catalog, your last answer has mistakes, which the user sees marked: " + + String.join("; ", found) + ". Do not repeat them."; + } + + /** The fenced code blocks of an answer; one whose fence is not closed (the model ran out of tokens) is left out. */ + private static List<CodeBlock> codeBlocks(String markdown) { + List<CodeBlock> answer = new ArrayList<>(); + String[] lines = markdown.split("\n", -1); + String language = null; + StringBuilder code = null; + for (int i = 0; i < lines.length; i++) { + String trimmed = lines[i].trim(); + if (trimmed.startsWith("```")) { + if (code == null) { + String tag = trimmed.substring(3).trim().toLowerCase(Locale.ROOT); + language = tag.contains(" ") ? tag.substring(0, tag.indexOf(' ')) : tag; + code = new StringBuilder(); + } else { + answer.add(new CodeBlock(answer.size() + 1, language, code.toString(), i)); + code = null; + } + } else if (code != null) { + code.append(lines[i]).append('\n'); + } + } + return answer; + } + + /** + * The camel.* keys of the properties lines of a block that the catalog does not know, in a block of any language: a + * model writes application.properties lines in a yaml block too. + */ + private static List<Mark> propertyMarks(CodeBlock block, CamelCatalog catalog) { + List<Mark> marks = new ArrayList<>(); + String[] lines = block.code().split("\n", -1); + for (int i = 0; i < lines.length; i++) { + Matcher key = CAMEL_PROPERTY.matcher(lines[i]); + if (!key.find() || OTHER_PROPERTIES.stream().anyMatch(key.group(1)::startsWith)) { + continue; + } + String msg = PropertiesChecks.validatePropertyLine(lines[i].strip(), catalog, null); + Matcher unknown = msg != null ? UNKNOWN_PROPERTY.matcher(msg) : null; + if (unknown != null && unknown.find()) { + String what = unknown.group(1).equals("option") + ? "a Camel option" : "a Camel option: there is no such " + unknown.group(1); + marks.add(new Mark(block.number(), i + 1, "`" + key.group(1) + "` is not " + what + hint(unknown.group(2)))); + } + } + return marks; + } + + /** The keys, options and endpoint options of a YAML block that do not exist; nothing when it is not Camel YAML. */ + private static List<Mark> yamlMarks(CodeBlock block, CamelCatalog catalog) throws Exception { + YamlFragment fragment = YamlFragment.of(block.code()); + if (fragment == null) { + return List.of(); + } + List<Mark> marks = new ArrayList<>(); + for (Error error : SourceValidator.yamlValidator(catalog).validate(fragment.content(), ProjectFacts.NONE)) { + Mark mark = schemaMark(block.number(), fragment, error); + if (mark != null) { + marks.add(mark); + } + } + for (String msg : EndpointChecks.validateYamlEndpoints(fragment.content(), catalog)) { + Matcher m = UNKNOWN_ENDPOINT_OPTION.matcher(msg); + int line = m.matches() ? Integer.parseInt(m.group(1)) - fragment.shift() : 0; + if (line > 0) { + marks.add(new Mark( + block.number(), line, + "`" + m.group(3) + "` is not an option of the `" + m.group(2) + "` component" + hint(m.group(4)))); + } + } + return marks; + } + + /** + * The mark of an error of the schema, or null when it is no mistake of the block: an unknown key where the wrapping + * put the block, a required key it leaves out, a value of the wrong type the validator has no hint for (a + * placeholder often is one). + */ + private static Mark schemaMark(int block, YamlFragment fragment, Error error) { + String location = String.valueOf(error.getInstanceLocation()); + String message = error.getMessage(); + if (message == null || fragment.items().matcher(location).matches()) { + return null; + } + String text = null; + if ("additionalProperties".equals(error.getKeyword())) { + Matcher key = UNKNOWN_KEY.matcher(message); + int after = message.indexOf("additional properties"); + if (key.find()) { + text = "`" + key.group(1) + "` is not " + owner(location) + + (after >= 0 ? hint(message.substring(after + "additional properties".length())) : ""); + } + } else if ("type".equals(error.getKeyword())) { + // a hint of the validator in place of the message (expression: holds the language as its key, ...) or after + // it (onException is a map, not a list, ...); a bare "string found, object expected" is left out, and so is + // null, the value of a key a fragment leaves empty (steps: with a comment under it) + Matcher found = TYPE_FOUND.matcher(message); + text = !found.matches() ? message : "null".equals(found.group(1)) ? null : found.group(2); + } + if (text == null) { + return null; + } + return new Mark(block, Math.max(YamlValidator.lineOf(fragment.content(), error) - fragment.shift(), 0), text); + } + + /** What an unknown key is not a part of: an option of the EIP at the location, or an EIP for an item of steps. */ + private static String owner(String location) { + String[] parts = location.split("/"); + String last = parts.length > 0 ? parts[parts.length - 1] : ""; + if (!last.matches("\\d+")) { + return "an option of `" + last + "`"; + } + String list = parts.length > 1 ? parts[parts.length - 2] : ""; + return switch (list) { + case "steps" -> "an EIP"; + case "beans" -> "a key of a bean"; + default -> "a key of the items of `" + list + "`"; + }; + } + + /** + * The hint after what a check says, as a mark writes it: " (did you mean `delete`?)" for the catalog's ". Did you + * mean: [delete]", the check's own words in parentheses, or nothing. + */ + private static String hint(String rest) { + // the schema's "did you mean 'backOffMultiplier'?" as code, as the names of the mark are + String text = rest == null ? "" : rest.strip().replaceAll("did you mean '([\\w.-]+)'\\?", "did you mean `$1`?"); + Matcher dym = DID_YOU_MEAN.matcher(text); + if (dym.matches()) { + List<String> names = new ArrayList<>(); + for (String name : dym.group(1).split(",")) { + if (!name.isBlank()) { + names.add("`" + name.strip() + "`"); + } + } + String more = dym.group(2).strip(); + return (names.isEmpty() ? "" : " (did you mean " + String.join(" or ", names) + "?)") + + (more.isEmpty() ? "" : " " + more); + } + if (text.isEmpty()) { + return ""; + } + return text.startsWith("(") ? " " + text : " (" + text.replaceFirst("^[.,:;]\\s*", "") + ")"; + } + /** * The {@code ${...}} placeholders of a text, braces balanced so {@code ${header.${header.key}}} is one placeholder, * in the order they appear. diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/EndpointChecks.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/EndpointChecks.java index f065e545f227..443ed2877fc7 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/EndpointChecks.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/EndpointChecks.java @@ -187,7 +187,9 @@ final class EndpointChecks { int mapIndent = -1; for (int k = j + 1; k < lines.length; k++) { String paramLine = lines[k]; - if (paramLine.isBlank()) { + if (paramLine.isBlank() || paramLine.trim().startsWith("#")) { + // a commented-out option is not one: "# idempotentRepository: ..." was reported as the + // unknown option '# idempotentRepository' (CAMEL-26960) continue; } int paramIndent = countLeadingSpaces(paramLine); diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/YamlFragment.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/YamlFragment.java new file mode 100644 index 000000000000..a910be2ccd00 --- /dev/null +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/YamlFragment.java @@ -0,0 +1,145 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.dsl.jbang.core.commands.ai; + +import java.util.Map; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * A YAML code block of an AI answer, made into something the YAML DSL schema can check (CAMEL-26960). The code of an + * answer is mostly a fragment: a {@code from:} without its route, a list of steps, one EIP. Checked as a route file, + * the schema refuses its shape before it looks at its keys, so the fragment is wrapped in what it is a part of: an + * entry of a route file goes in a file, steps and an EIP go in a route, and an option of one EIP (redeliveryPolicy:) + * goes under that EIP. An option map without its key (slidingWindowSize: 10) is not checked: what it belongs to is not + * known. + * + * @param content the YAML the schema checks: the block as it is for a route file, else the block in its wrapping + * @param shift the lines the wrapping puts before the first line of the block, to subtract from a line of an error + * @param items the locations of the items that hold the block: an unknown key there says only that the block is not + * what it was taken for (options of an EIP shown without the EIP: slidingWindowSize: 10), so it is not a + * mistake of the block + */ +record YamlFragment(String content, int shift, Pattern items) { + + /** The key of the first line: an item of a list (- from:) or a key of a mapping (from:). */ + private static final Pattern FIRST_KEY = Pattern.compile("^(- )?([A-Za-z_][\\w.-]*)\\s*:"); + + /** The top-level keys of YAML that is not Camel: docker-compose, Kubernetes, Spring Boot and Quarkus settings. */ + private static final Set<String> OTHER_YAML = Set.of("apiVersion", "kind", "metadata", "services", "version", + "spring", "quarkus", "camel", "server", "management", "logging", "jobs", "on"); + + /** The route that steps and an EIP are put in: they are checked as the steps of its from:. */ + private static final String ROUTE = "- route:\n from:\n uri: direct:answer\n steps:\n"; + + private static final Pattern FILE_ITEMS = Pattern.compile("/\\d+"); + private static final Pattern ENTRY_ITEMS = Pattern.compile("/0"); + private static final Pattern STEP_ITEMS = Pattern.compile("/0(/route(/from(/steps/\\d+)?)?)?"); + + /** + * The keys that are neither an entry nor a step but an option of one EIP, which a model shows on their own + * (redeliveryPolicy: with its options), and the wrapping that puts them under that EIP: what goes before the block, + * how far the block is indented, and the locations of the wrapping. + */ + private record Parent(String before, String indent, Pattern items) { + } + + private static final Parent ON_EXCEPTION = new Parent( + "- onException:\n exception:\n - java.lang.Exception\n", " ", + Pattern.compile("/0(/onException(/exception(/\\d+)?)?)?")); + + private static final Parent CIRCUIT_BREAKER = new Parent( + ROUTE + " - circuitBreaker:\n", " ", + Pattern.compile("/0(/route(/from(/steps(/0(/circuitBreaker)?)?)?)?)?")); + + private static final Map<String, Parent> PARENTS = Map.of( + "redeliveryPolicy", ON_EXCEPTION, + "resilience4jConfiguration", CIRCUIT_BREAKER, + "faultToleranceConfiguration", CIRCUIT_BREAKER, + "onFallback", CIRCUIT_BREAKER); + + /** + * The block made checkable, or null when it is not Camel YAML: it has no key, or its first key is one of another + * tool (services:, apiVersion:, spring:). + */ + static YamlFragment of(String code) { + String[] lines = code.split("\n", -1); + int indent = Integer.MAX_VALUE; + String first = null; + for (String line : lines) { + if (!line.isBlank() && !line.strip().startsWith("#")) { + indent = Math.min(indent, YamlLines.countLeadingSpaces(line)); + if (first == null) { + first = line; + } + } + } + if (first == null) { + return null; + } + // a block cut out of a deeper level of a route: its lines as if it were at the top + String[] dedented = new String[lines.length]; + for (int i = 0; i < lines.length; i++) { + dedented[i] = YamlLines.countLeadingSpaces(lines[i]) >= indent ? lines[i].substring(indent) : lines[i].strip(); + } + Matcher m = FIRST_KEY.matcher(first.substring(indent)); + if (!m.find() || OTHER_YAML.contains(m.group(2))) { + return null; + } + boolean item = m.group(1) != null; + if (CatalogSamples.TOP_LEVEL.contains(m.group(2))) { + // a route file (- route:, - beans:, - onException:), or one of its entries written as a mapping (from:) + return item + ? new YamlFragment(String.join("\n", dedented), 0, FILE_ITEMS) + : new YamlFragment(indented(dedented, "- ", " "), 0, ENTRY_ITEMS); + } + Parent parent = item ? null : PARENTS.get(m.group(2)); + if (parent != null) { + // redeliveryPolicy: under an onException, resilience4jConfiguration: under a circuitBreaker + return new YamlFragment( + parent.before() + indented(dedented, parent.indent(), parent.indent()), + (int) parent.before().chars().filter(c -> c == '\n').count(), parent.items()); + } + // steps (- to:, - log:), or one EIP (idempotentConsumer:): the steps of a route + return item + ? new YamlFragment(ROUTE + indented(dedented, " ", " "), 4, STEP_ITEMS) + : new YamlFragment(ROUTE + indented(dedented, " - ", " "), 4, STEP_ITEMS); + } + + /** + * The lines with first in front of the first line that has a key and other in front of the others, so a mapping + * becomes an item of a list, or a list moves in under the key that holds it. The lines stay where they are, so a + * line of the content is a line of the block plus {@link #shift()}. + */ + private static String indented(String[] lines, String first, String other) { + StringBuilder sb = new StringBuilder(); + boolean keyed = false; + for (int i = 0; i < lines.length; i++) { + String line = lines[i]; + if (!line.isEmpty()) { + boolean key = !keyed && !line.isBlank() && !line.strip().startsWith("#"); + sb.append(key ? first : other).append(line); + keyed |= key; + } + if (i < lines.length - 1) { + sb.append('\n'); + } + } + return sb.toString(); + } +} diff --git a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/AskLoopTest.java b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/AskLoopTest.java index 0f591b63b636..d686ffa9e5ae 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/AskLoopTest.java +++ b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/AskLoopTest.java @@ -107,6 +107,20 @@ class AskLoopTest extends CamelCommandBaseTestSupport { assertThat(printer.getStderr()).contains("Reached maximum iterations (2) without a final answer."); } + @Test + void theMistakesOfTheCodeOfTheAnswerAreSaidUnderIt() throws Exception { + // CAMEL-26960: an option that does not exist, on stderr, so the answer stays the only output on stdout + String text = "Delete the files after reading them:\n\n```yaml\nfrom:\n uri: file:orders\n parameters:\n" + + " deleteAfterRead: true\n```\n"; + server.reply("/api/chat", 200, answer(text)); + + assertThat(command().doCall()).isZero(); + + assertThat(printer.getStdout()).isEqualTo(text.strip()); + assertThat(printer.getStderr()).contains("Check: code block 1, line 4: `deleteAfterRead` is not an option of" + + " the `file` component (did you mean `delete`?)"); + } + @Test void theStatsOnStderr() throws Exception { server.reply("/api/chat", 200, answer("The routes are fine")); diff --git a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/AnswerCodeChecksTest.java b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/AnswerCodeChecksTest.java new file mode 100644 index 000000000000..b2efe6fe73e6 --- /dev/null +++ b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/AnswerCodeChecksTest.java @@ -0,0 +1,260 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.dsl.jbang.core.commands.ai; + +import java.util.List; + +import org.apache.camel.catalog.CamelCatalog; +import org.apache.camel.catalog.DefaultCamelCatalog; +import org.apache.camel.dsl.jbang.core.commands.ai.AnswerChecks.Mark; +import org.apache.camel.dsl.jbang.core.commands.ai.AnswerChecks.Problem; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.tuple; + +/** + * CAMEL-26960: the code blocks of an answer are checked for what does not exist, and the mistakes are marked under + * their block. The snippets are from a benchmark with a local model (qwen3.6:35b, November 2026), which wrote them in + * its answers to beginner questions. + */ +class AnswerCodeChecksTest { + + private static final CamelCatalog CATALOG = new DefaultCamelCatalog(); + + @Test + void aFromWithoutItsRouteIsChecked() { + List<Mark> marks = AnswerChecks.checkCode(answer("yaml", """ + from: + uri: file:orders + steps: + - idempotentConsumer: + simple: "${header.CamelFileName}" + messageIdRepository: orderIds + steps: + - to: direct:audit + """), CATALOG); + assertThat(marks).extracting(Mark::block, Mark::line).containsExactly(tuple(1, 6)); + assertThat(marks.get(0).describe()) + .startsWith("code block 1, line 6: `messageIdRepository` is not an option of `idempotentConsumer`"); + } + + @Test + void stepsAndOneEipAreCheckedInARoute() { + List<Mark> marks = AnswerChecks.checkCode(answer("yaml", """ + - circuitBreaker: + forceOpen: false + failureThreshold: 50 + steps: + - to: direct:supplier + """), CATALOG); + assertThat(marks).extracting(Mark::line).containsExactly(2, 3); + assertThat(marks.get(0).message()).isEqualTo("`forceOpen` is not an option of `circuitBreaker`"); + // the validator's hint comes with the mark + assertThat(marks.get(1).message()).startsWith("`failureThreshold` is not an option of `circuitBreaker` (") + .contains("resilience4jConfiguration"); + + marks = AnswerChecks.checkCode(answer("yaml", """ + idempotentConsumer: + simple: "${body[orderId]}" + messageIdRepository: orderIds + """), CATALOG); + assertThat(marks).extracting(Mark::line).containsExactly(3); + } + + @Test + void anOptionOfOneEipIsCheckedUnderIt() { + List<Mark> marks = AnswerChecks.checkCode(answer("yaml", """ + redeliveryPolicy: + maximumRedeliveries: 3 + redeliveryDelay: 1000 + multiplier: 2 # 1s, 2s, 4s + useExponentialBackOff: true + """), CATALOG); + assertThat(marks).extracting(Mark::line).containsExactly(4); + assertThat(marks.get(0).message()).startsWith("`multiplier` is not an option of `redeliveryPolicy`"); + } + + @Test + void theOptionsOfAnEndpointAreChecked() { + List<Mark> marks = AnswerChecks.checkCode(answer("yaml", """ + from: + uri: file:orders + parameters: + noop: true + deleteAfterRead: true + steps: + - to: "direct:audit?retryCount=3" + """), CATALOG); + assertThat(marks).extracting(Mark::line, Mark::message).containsExactly( + tuple(5, "`deleteAfterRead` is not an option of the `file` component (did you mean `delete`?)"), + tuple(7, "`retryCount` is not an option of the `direct` component")); + + // an option that is commented out is not one + assertThat(AnswerChecks.checkCode(answer("yaml", """ + from: + uri: file + parameters: + directoryName: orders + # idempotentRepository: "#redisRepo" + steps: + - log: "${body}" + """), CATALOG)).isEmpty(); + } + + @Test + void theCamelPropertiesAreCheckedInAnyBlock() { + // a model writes application.properties lines in a yaml block too + List<Mark> marks = AnswerChecks.checkCode(answer("yaml", """ + # Add to application.properties + camel.server.context-path=/api + camel.jbang.dependencies=org.example:orders:1.0 + orders.directory=orders + """), CATALOG); + assertThat(marks).extracting(Mark::line).containsExactly(2); + assertThat(marks.get(0).message()).startsWith("`camel.server.context-path` is not a Camel option (") + .contains("camel.rest.contextPath"); + + marks = AnswerChecks.checkCode(answer("properties", """ + camel.main.name=orders + camel.component.file.orders-directory=orders + """), CATALOG); + assertThat(marks).extracting(Mark::line, Mark::message) + .containsExactly(tuple(2, "`camel.component.file.orders-directory` is not a Camel option")); + } + + @Test + void anOnExceptionWrittenAsAListIsMarked() { + // the exception class as a key: the validator knows the mistake, and says how to write it + List<Mark> marks = AnswerChecks.checkCode(answer("yaml", """ + onException: + - java.lang.Exception: + handled: true + steps: + - to: direct:error-handler + """), CATALOG); + assertThat(marks).extracting(Mark::line).containsExactly(2); + assertThat(marks.get(0).message()).startsWith("onException is a map, not a list"); + } + + @Test + void whatIsNotAMistakeIsNotMarked() { + // a route as the examples have it + assertThat(marks("yaml", """ + - route: + id: orders + from: + uri: timer:orders?period=5000 + steps: + - setBody: + simple: "order ${random(1000)}" + - log: "${body}" + """)).isEmpty(); + // options shown without the EIP they are of: what they belong to is not known + assertThat(marks("yaml", """ + slidingWindowSize: 10 + minimumNumberOfCalls: 5 + """)).isEmpty(); + // a required option and the steps left out, and a placeholder + assertThat(marks("yaml", """ + - from: + uri: timer + steps: + - to: "{{orders.target}}" + - choice: + otherwise: + steps: + # your steps here + """)).isEmpty(); + // the YAML of other tools + assertThat(marks("yaml", """ + services: + kafka: + image: quay.io/strimzi/kafka:latest + """)).isEmpty(); + assertThat(marks("yaml", """ + apiVersion: v1 + kind: ConfigMap + metadata: + name: orders + """)).isEmpty(); + assertThat(marks("yaml", """ + camel: + springboot: + name: orders + """)).isEmpty(); + // a message body, and a block of another language + assertThat(marks("yaml", """ + "lines": [{"sku": "CAMEL-TSHIRT", "qty": 1}] + """)).isEmpty(); + assertThat(marks("text", """ + redeliveryPolicy: + multiplier: 2 + """)).isEmpty(); + // a block the model did not finish + assertThat(AnswerChecks.checkCode("Retry with:\n```yaml\nredeliveryPolicy:\n multiplier: 2\n", CATALOG)) + .isEmpty(); + } + + @Test + void theMarksGoUnderTheirBlock() { + String answer = """ + Two changes: + + 1. Keep the order files: + + ```yaml + from: + uri: file:orders?noop=true + ``` + 2. Retry the call: + + ```yaml + redeliveryPolicy: + multiplier: 2 + ``` + That is all."""; + List<Mark> marks = AnswerChecks.checkCode(answer, CATALOG); + assertThat(marks).extracting(Mark::block, Mark::line).containsExactly(tuple(2, 2)); + + String marked = AnswerChecks.withMarks(answer, marks); + // the first block is as it was, and the mark of the second is under it, indented as its fence + assertThat(marked).startsWith(answer.substring(0, answer.indexOf("2. Retry"))); + assertThat(marked).endsWith(" multiplier: 2\n ```\n\n" + + " ā line 2: `multiplier` is not an option of `redeliveryPolicy`\n\nThat is all."); + assertThat(AnswerChecks.withMarks(answer, List.of())).isEqualTo(answer); + } + + @Test + void theModelIsToldWhatIsLeft() { + Mark mark = new Mark(2, 4, "`multiplier` is not an option of `redeliveryPolicy`"); + Problem problem = new Problem("${header.user ?: Guest}", "Other operator ?: does not accept Guest"); + assertThat(AnswerChecks.historyNote(List.of(mark), List.of(problem))) + .contains("code block 2, line 4: `multiplier` is not an option of `redeliveryPolicy`") + .contains("invalid simple expression ${header.user ?: Guest}: Other operator ?: does not accept Guest") + .endsWith("Do not repeat them."); + assertThat(AnswerChecks.historyNote(List.of(), List.of())).isNull(); + } + + private static List<Mark> marks(String language, String code) { + return AnswerChecks.checkCode(answer(language, code), CATALOG); + } + + private static String answer(String language, String code) { + return "Here is how:\n\n```" + language + "\n" + code + "```\n\nThat should do it."; + } +} diff --git a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/SourceValidatorEndpointTest.java b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/SourceValidatorEndpointTest.java index 61b5a6f5f4f7..48ff64197625 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/SourceValidatorEndpointTest.java +++ b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/SourceValidatorEndpointTest.java @@ -102,6 +102,23 @@ class SourceValidatorEndpointTest { assertThat(errors.get(0)).containsIgnoringCase("unknown"); } + @Test + void aCommentedOutOptionIsNotAnOption() { + // CAMEL-26960: was the unknown option '# idempotentRepository' + String yaml = """ + - from: + uri: file + parameters: + directoryName: orders + noop: true + # or + # idempotentRepository: "#redisRepo" + steps: + - log: "${body}" + """; + assertThat(SourceValidator.validateYamlEndpoints(yaml, catalog)).isEmpty(); + } + @Test void expandedFormInvalidBoolean() { String yaml = """ diff --git a/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanel.java b/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanel.java index 715e7068533d..02267a7c5765 100644 --- a/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanel.java +++ b/dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanel.java @@ -1434,14 +1434,20 @@ class AiPanel { if (text != null && !text.isBlank()) { long elapsed = MonotonicClock.millis() - thinkingStartTime; String note = problems.isEmpty() ? null : answerNote(problems); + // the code of the answer is checked too: what does not exist is marked under its block, and the model is + // told of it, and of a simple expression left invalid, with the next question (CAMEL-26960) + List<AnswerChecks.Mark> marks = AnswerChecks.checkCode(text); + String told = AnswerChecks.historyNote(marks, problems); + pendingNote = told == null ? pendingNote : pendingNote == null ? told : pendingNote + "\n" + told; + String shown = AnswerChecks.withMarks(text, marks); ConversationEntry entry = new ConversationEntry( - AiRole.ASSISTANT, text, elapsed, turnAiMs, turnToolMs, turnToolCalls, + AiRole.ASSISTANT, shown, elapsed, turnAiMs, turnToolMs, turnToolCalls, totalUsage.totalTokens(), turnRequests, wrapUp, contextFillPercent(), corrections, note); conversation.add(entry); usage.addTurn(turnAiMs, turnToolMs, turnToolCalls); String label = wrapUp ? "Response after reaching the tool call limit (" : "Response ("; log(LogLevel.RESPONSE, label + entry.byline() + describeCacheSignal(totalUsage) + ")", - note == null ? text : text + "\n\n" + note); + note == null ? shown : shown + "\n\n" + note); } else { String err = "Empty response from LLM."; conversation.add(new ConversationEntry(AiRole.ERROR, err)); diff --git a/dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanelTest.java b/dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanelTest.java index 1f50075211ab..00770014ff65 100644 --- a/dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanelTest.java +++ b/dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanelTest.java @@ -1200,6 +1200,69 @@ class AiPanelTest { assertFalse(last.byline().contains("correction"), last.byline()); } + // ---- the code of an answer is checked, and what does not exist is marked (CAMEL-26960) ---- + + @Test + void theMistakesOfTheCodeOfAnAnswerAreMarkedAndToldWithTheNextQuestion() throws Exception { + AiPanel panel = new AiPanel(); + panel.setToolRegistryForTesting(new TuiToolRegistry(null)); + MarkedLlmClient client = new MarkedLlmClient(); + panel.setClientForTesting(client); + panel.open(); + type(panel, "how do I retry the call"); + panel.handleKeyEvent(KeyEvent.ofKey(KeyCode.ENTER, KeyModifiers.NONE)); + await().atMost(10, TimeUnit.SECONDS).until(() -> !panel.isAgentThreadRunningForTesting()); + + AiPanel.ConversationEntry last = panel.conversationForTesting().get(panel.conversationForTesting().size() - 1); + assertEquals(AiRole.ASSISTANT, last.role(), last.text()); + assertTrue(last.text().contains("```\n\nā line 4: `multiplier` is not an option of `redeliveryPolicy`"), + last.text()); + assertEquals(1, client.answers, "a mark is not a correction turn: the model is not asked again"); + assertEquals(0, last.corrections()); + + // the model is told with the next question; its answer stays in the history as it wrote it + type(panel, "and what else"); + panel.handleKeyEvent(KeyEvent.ofKey(KeyCode.ENTER, KeyModifiers.NONE)); + await().atMost(10, TimeUnit.SECONDS).until(() -> !panel.isAgentThreadRunningForTesting()); + assertTrue(client.lastQuestion.contains("[Checked against the Camel catalog, your last answer has mistakes"), + client.lastQuestion); + assertTrue(client.lastQuestion.contains("code block 1, line 4: `multiplier` is not an option of"), + client.lastQuestion); + assertTrue(client.lastQuestion.endsWith("and what else"), client.lastQuestion); + assertEquals(MarkedLlmClient.ANSWER, client.previousAnswer); + assertTrue(panel.conversationForTesting().stream().noneMatch(e -> e.role() == AiRole.SYSTEM), + "the marks are under the answer already, so no line says that the model is told"); + } + + /** Answers with a redelivery policy that has an option no redelivery policy has, and keeps what it is sent. */ + private static final class MarkedLlmClient extends LlmClient { + + static final String ANSWER = "Retry three times:\n\n```yaml\nredeliveryPolicy:\n maximumRedeliveries: 3\n" + + " redeliveryDelay: 1000\n multiplier: 2\n```\n"; + + volatile int answers; + volatile String lastQuestion; + volatile String previousAnswer; + + MarkedLlmClient() { + withModel("test-model"); + withApiType(ApiType.openai); + } + + @Override + public boolean detectEndpoint() { + return true; + } + + @Override + public ChatResponse chatWithTools(String systemPrompt, List<Message> messages, List<ToolDef> tools) { + answers++; + lastQuestion = messages.get(messages.size() - 1).content(); + previousAnswer = messages.size() >= 2 ? messages.get(messages.size() - 2).content() : null; + return new ChatResponse(answers == 1 ? ANSWER : "Nothing else.", List.of(), "stop", false, TokenUsage.EMPTY); + } + } + /** Answers with a fallback that is not quoted, and fixes it when asked, like a small local model. */ private static final class CorrectingLlmClient extends LlmClient { diff --git a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/SchemaHints.java b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/SchemaHints.java index 2fe2487f41a4..080e10afbda4 100644 --- a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/SchemaHints.java +++ b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/SchemaHints.java @@ -135,6 +135,12 @@ final class SchemaHints { private static final Predicate<Match> ANY = m -> true; + /** Whether the node of the error, an item of beans, has its name: key. */ + private static boolean hasName(Match m) { + JsonNode instance = m.error().getInstanceNode(); + return instance != null && instance.has("name"); + } + /** {sku} when the node is the map YAML reads an unquoted {sku} as (one key, no value); null otherwise. */ static String unquotedBraces(Match m) { JsonNode instance = m.error().getInstanceNode(); @@ -870,8 +876,14 @@ final class SchemaHints { unknownProperty(".*/beans/\\d+", m -> Set.of("id", "ref", "class").contains(m.unknown()), m -> "a bean is - name: myBean followed by type: \"#class:com.example.MyBean\" (name instead of " + m.unknown() + (m.unknown().equals("class") ? ", type instead of class" : "") + ")"), - // - myBean: {type: ...} instead of - name: myBean / type: ... - unknownProperty(".*/beans/\\d+", ANY, + // - name: repo / type: ... / parameters: {...}: the options of a bean are its properties (CAMEL-26960) + unknownProperty(".*/beans/\\d+", + m -> hasName(m) && Set.of("parameters", "params", "options", "args").contains(m.unknown()), + m -> "what a bean is set up with goes under properties: (its setters, properties: {fileStore: x})" + + " or constructors: (the arguments of its constructor, constructors: {0: x})"), + // - myBean: {type: ...} instead of - name: myBean / type: ...; not for an item that has its name, where the + // key is one a bean does not have + unknownProperty(".*/beans/\\d+", m -> !hasName(m), m -> "a bean item is written as - name: " + m.unknown() + " followed by type: \"#class:com.example.MyBean\" (the name is a property, not the key)"), // errorHandler: under from: belongs to the route; told only that it is a top-level entry, models moved a diff --git a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/YamlValidatorPropertyHintTest.java b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/YamlValidatorPropertyHintTest.java index fc5aeb0052c5..8ad5674c90a5 100644 --- a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/YamlValidatorPropertyHintTest.java +++ b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/YamlValidatorPropertyHintTest.java @@ -686,6 +686,31 @@ public class YamlValidatorPropertyHintTest { assertThat(errors).anyMatch(e -> e.getMessage().contains("a bean item is written as - name: myBean")); } + @Test + public void testBeanWithItsNameAndAnUnknownKeyIsNotTakenForAKeyedItem() throws Exception { + // CAMEL-26960: a local model wrote the setters of a bean under parameters:, and was told that its name is a + // property and not the key, which it already was + List<Error> errors = validator.validate(""" + - beans: + - name: orderIds + type: org.apache.camel.processor.idempotent.FileIdempotentRepository + parameters: + fileStore: orders.dat + """); + assertThat(errors).anyMatch(e -> e.getMessage().contains("property 'parameters' is not defined") + && e.getMessage().contains("goes under properties:")); + assertThat(errors).noneMatch(e -> e.getMessage().contains("a bean item is written as")); + + errors = validator.validate(""" + - beans: + - name: orderIds + type: org.apache.camel.processor.idempotent.FileIdempotentRepository + fileStore: orders.dat + """); + assertThat(errors).anyMatch(e -> e.getMessage().contains("property 'fileStore' is not defined")); + assertThat(errors).noneMatch(e -> e.getMessage().contains("a bean item is written as")); + } + @Test public void testTemplateBeansKeyedByNameSayHowToWriteThem() throws Exception { // CAMEL-24704: the beans of a routeTemplate and a templatedRoute are the same list as the top-level beans
