This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch CAMEL-26758 in repository https://gitbox.apache.org/repos/asf/camel.git
commit 8c3a5701345da0e308b17e2d8dc2a4a1afd5818f Author: Claus Ibsen <[email protected]> AuthorDate: Mon Nov 9 08:51:14 2026 +0100 CAMEL-26758: camel-jbang - AI tools: the third identical refused write gets the fix that the refusal gave A local model sometimes sends the same refused file again and again, also when the refusal gives the exact lines to write. In r28 to r30 of the local-model benchmark, 13 of the 14 step runs with 4 or more such sends failed, and all 9 with 8 or more. Repairs per mistake (CAMEL-25532, CAMEL-26730) moved the loop to the next mistake: contracts-rest-code-first step 1 looped on a concatenation and an extra quote (r28), the literal \n (r29) and a jsonpath with [] (r30, 15 times in two runs). When the write tool refuses the same content of a file for the third time in a row, it now makes the fixes the refusals gave as exact text ("Write lines A-B as:" with the lines, or "write the line as ..."), repairs the shapes with one reading, and writes the file only when no problem is left; the answer lists what changed and says to read the file before the next edit. A problem without such text must be gone after the fixes, else nothing is written. Not for Citrus tests. The edit tool gets it too. The jsonpath message for [] and the message for an unclosed double quote now end with the line to write, so the write tool can apply them. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_016Y4dDi5iQNgtHrDdz2NpwF --- .../dsl/jbang/core/commands/ai/AuthoringTools.java | 63 ++++++++- .../dsl/jbang/core/commands/ai/JsonPathChecks.java | 18 ++- .../dsl/jbang/core/commands/ai/RefusalFixes.java | 152 +++++++++++++++++++++ .../ai/AuthoringToolsRefusalFixesTest.java | 139 +++++++++++++++++++ .../jbang/core/commands/ai/JsonPathChecksTest.java | 7 +- .../jbang/core/commands/ai/RefusalFixesTest.java | 113 +++++++++++++++ .../camel/dsl/yaml/validator/YamlValidator.java | 9 +- .../validator/YamlValidatorPropertyHintTest.java | 3 +- 8 files changed, 492 insertions(+), 12 deletions(-) diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/AuthoringTools.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/AuthoringTools.java index 53396c2afd8b..aa4ddfc7457f 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/AuthoringTools.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/AuthoringTools.java @@ -1733,6 +1733,7 @@ public final class AuthoringTools { List<String> problemsBefore = List.of(); List<String> repaired = List.of(); List<String> repairable = List.of(); + boolean refusalsFixed = false; // a Citrus test is not a route: none of the repairs, notes and reloads of a route apply to it boolean citrusTest = SourceValidator.isCitrusTest(file); if (validate && SourceValidator.isValidatableFile(file)) { @@ -1765,6 +1766,19 @@ public final class AuthoringTools { repairable = markShapeLeft(r); } } + int times = errors.isEmpty() ? 0 : refusedAgain(path, content); + if (times >= SAME_REFUSALS_FIXED && !citrusTest) { + // CAMEL-26758: the same refused content a third time, also when the refusals gave the lines to write; + // the write makes the fixes they gave as exact text, and goes ahead when that leaves no new problem + Repaired given = refusalFixes(ctx, dir, file, content, errors, before, exists); + if (given != null) { + content = given.content(); + repaired = given.applied(); + problemsBefore = given.stillThere(); + errors = List.of(); + refusalsFixed = true; + } + } if (!errors.isEmpty()) { JsonObject result = new JsonObject(); result.put("status", "invalid"); @@ -1772,7 +1786,6 @@ public final class AuthoringTools { result.put("errors", new JsonArray(errors)); result.put("message", "The file was not written: the content has validation errors. Fix them and" + " call camel_write_file again." + startNote(dir, file, content)); - int times = refusedAgain(path, content); if (times > 1) { // the same content again: a model that does not see the problem in its own lines writes them // again and again, sixteen times in a row in the benchmark; say so, and what to do instead @@ -1951,7 +1964,13 @@ public final class AuthoringTools { if (!repaired.isEmpty()) { result.put("repaired", new JsonArray(repaired)); result.put("message", "Written with these repairs of the YAML (the file has them now): " - + String.join("; ", repaired) + ". " + result.getString("message")); + + String.join("; ", repaired) + ". " + + (refusalsFixed + ? "The same content came a third time, so the write made the fixes the" + + " refusals gave: the file is not the text you sent, read it before you" + + " edit it. " + : "") + + result.getString("message")); } if (!unusedProperties.isEmpty()) { // what to do next, first in the answer: "Use {{report.schedule}} again on line 7, and set @@ -2029,6 +2048,46 @@ public final class AuthoringTools { return applied.isEmpty() ? null : new Repaired(current, applied, stillThere, left, moved); } + /** + * How many times in a row the same content of a file is refused before the write makes the fixes the refusals gave + * (CAMEL-26758): twice the model had the choice of its own fix. In the benchmark, of the loops that stopped at the + * third refusal most passed after it, while 13 of the 14 that went on to a fourth failed. + */ + private static final int SAME_REFUSALS_FIXED = 3; + + /** + * CAMEL-26758: the content with the fixes the refusals gave as exact text ({@link RefusalFixes}), and then the + * shapes with one reading repaired; null when no problem gives exact text, or a problem is still left after the + * fixes (a problem without text often came from a line a fix rewrote, and goes with it). + */ + private static Repaired refusalFixes( + ToolContext ctx, Path dir, String file, String content, List<String> errors, List<String> before, + boolean exists) { + LenientRepairs.Result given = RefusalFixes.apply(content, errors); + if (given == null) { + return null; + } + List<String> applied = new ArrayList<>(given.applied()); + String fixed = given.content(); + List<String> left = SourceValidator.validate(file, fixed, ctx.catalog(), ctx.propertyLineValidator(), dir, null, + false); + List<String> stillThere = new ArrayList<>(); + if (!left.isEmpty() && exists) { + left = newProblems(before, left, stillThere); + } + if (!left.isEmpty()) { + // a fix can leave a shape with one reading, such as a list item one column off + Repaired r = repair(ctx, dir, file, fixed, left, before, exists); + if (r == null || !r.left().isEmpty()) { + return null; + } + fixed = r.content(); + applied.addAll(r.applied()); + stillThere = r.stillThere(); + } + return new Repaired(fixed, applied, stillThere, List.of(), given.moved()); + } + /** * The problems a repairable shape hid: the YAML parser stops at a list item two columns off, so the checks of the * content never ran. Told only of the column, a local model sent the same lines up to 22 times in a row (r23 and diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/JsonPathChecks.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/JsonPathChecks.java index ec377f3fa03f..f0bf47d6638b 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/JsonPathChecks.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/JsonPathChecks.java @@ -95,7 +95,7 @@ public final class JsonPathChecks { if (text.contains("[?(") || !COMPARISON.matcher(text).find() || predicate) { // a path, or a condition (the easy predicate syntax reads $.status == 'paid' as $[?(@.status == // 'paid')]): compiled as the runtime does - String compile = compileProblem(catalog, text, predicate); + String compile = compileProblem(catalog, text, predicate, lines[lineNum - 1]); if (compile != null) { errors.add("Line " + lineNum + ": " + compile); } @@ -168,6 +168,15 @@ public final class JsonPathChecks { * predicate syntax, which camel-jsonpath turns into a filter first */ static String compileProblem(CamelCatalog catalog, String text, boolean predicate) { + return compileProblem(catalog, text, predicate, null); + } + + /** + * As {@link #compileProblem(CamelCatalog, String, boolean)}, with the YAML line that has the expression: the note + * on [] ends with that line to write, which the write tool applies when the model sends the same lines a third time + * (CAMEL-26758; r30 rest-code-first step 1 sent ...')][] 15 times in two runs, told to write it without the []). + */ + static String compileProblem(CamelCatalog catalog, String text, boolean predicate, String line) { if (predicate && !text.contains("[?(") && COMPARISON.matcher(text).find()) { // the easy predicate syntax, which camel-jsonpath turns into a filter first return null; @@ -212,8 +221,11 @@ public final class JsonPathChecks { } String problem = "jsonpath " + text + " does not compile: " + why.strip(); if (EMPTY_BRACKETS.matcher(text).find()) { - return problem + "; JsonPath has no [], and a filter or [*] gives a list already: write " - + EMPTY_BRACKETS.matcher(text).replaceAll(""); + String fixed = EMPTY_BRACKETS.matcher(text).replaceAll(""); + // the whole line when it has the expression as written once; else the expression alone + boolean once = line != null && line.contains(text) && line.indexOf(text) == line.lastIndexOf(text); + return problem + "; JsonPath has no [], and a filter or [*] gives a list already: " + + (once ? "write the line as " + line.replace(text, fixed).strip() : "write " + fixed); } return problem; } diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/RefusalFixes.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/RefusalFixes.java new file mode 100644 index 000000000000..59ba1c1fe212 --- /dev/null +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/RefusalFixes.java @@ -0,0 +1,152 @@ +/* + * 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.ArrayList; +import java.util.Arrays; +import java.util.Comparator; +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * CAMEL-26758: the fixes the refusals of a write gave as exact text, made by the write tool when the model sends the + * same refused content a third time. A local model that does not see the problem in its own lines sends them again and + * again, also when the refusal says which lines to write: in r28 to r30 of the local-model benchmark, 13 of the 14 step + * runs with 4 or more such sends failed, and contracts-rest-code-first step 1 sent ...')][] 15 times in two runs of + * r30, told each time to write the jsonpath without the []. A repair per shape ({@link LenientRepairs}) moved the loop + * to the next mistake. Two forms of exact text end a message, and only those are applied: + * <ul> + * <li>the block form, {@code Write line 7 as:} or {@code Write lines 48-51 as:} (or {@code Write line 45 as these 2 + * lines:}), and then the lines with their indentation;</li> + * <li>the line form, {@code line 49: ... write the line as expression: "$.lines"}: the line without its indentation, + * which is kept.</li> + * </ul> + * A problem without such text is left to the check that follows: the caller validates the result again, and writes it + * only when no problem is left. So a problem that came from a line a fix rewrote goes with it (an unclosed quote also + * gives a Simple syntax error on its line), and any other keeps the write refused, with nothing written: the model + * never goes on from a file that is neither its own text nor the file on disk. + */ +final class RefusalFixes { + + /** The line a message starts with: "line 49: ...", "Line 46: ...", "line 37 (" - doCatch:"): ...". */ + private static final Pattern LEADING_LINE = Pattern.compile("^[Ll]ine (\\d+)\\b"); + /** The block form; its lines are the rest of the message. */ + private static final Pattern BLOCK = Pattern.compile( + "Write (?:line (\\d+)|lines (\\d+)-(\\d+)) as(?: these (\\d+) lines)?:\\n"); + private static final String LINE_FORM = "write the line as "; + /** A key of a YAML mapping: expression of expression: "x", to of - to: x. */ + private static final Pattern KEY = Pattern.compile("^([\\w.-]+):(?:\\s|$)"); + + private RefusalFixes() { + } + + /** The lines from (0-based) to to (exclusive) written as the message said. */ + private record Fix(int from, int to, List<String> written, String description) { + } + + /** + * The content with the fixes the problems gave as exact text made, and what was done, one line each ("line 49: + * written as the refusals said: ..."); null when no problem gives exact text that fits the content, or when two + * fixes are of the same lines. + */ + static LenientRepairs.Result apply(String content, List<String> errors) { + List<String> lines = new ArrayList<>(Arrays.asList(content.split("\n", -1))); + List<Fix> fixes = new ArrayList<>(); + for (String error : errors) { + Fix fix = fixOf(error, lines); + if (fix != null && !fixes.contains(fix)) { + fixes.add(fix); + } + } + if (fixes.isEmpty()) { + return null; + } + // from the bottom, so the line numbers of the ones above stay right + fixes.sort(Comparator.comparingInt(Fix::from).reversed()); + int below = Integer.MAX_VALUE; + for (Fix f : fixes) { + if (f.to() > below) { + return null; + } + below = f.from(); + } + List<String> applied = new ArrayList<>(); + boolean moved = false; + for (Fix f : fixes) { + List<String> range = lines.subList(f.from(), f.to()); + moved |= range.size() != f.written().size(); + range.clear(); + range.addAll(f.written()); + applied.add(0, f.description()); + } + return new LenientRepairs.Result(String.join("\n", lines), applied, moved); + } + + private static Fix fixOf(String error, List<String> lines) { + Matcher block = BLOCK.matcher(error); + if (block.find()) { + boolean one = block.group(1) != null; + int first = Integer.parseInt(one ? block.group(1) : block.group(2)); + int last = one ? first : Integer.parseInt(block.group(3)); + List<String> written = List.of(error.substring(block.end()).split("\n", -1)); + // "as these 2 lines": one line becomes several + int count = block.group(4) != null ? Integer.parseInt(block.group(4)) : last - first + 1; + if (first < 1 || last < first || last > lines.size() || written.size() != count + || block.group(4) != null && !one + // a long block is shown in part, with "(and the 3 lines after them, ...)" (IndentLines) + || written.stream().anyMatch(l -> l.startsWith("(and the "))) { + return null; + } + return new Fix( + first - 1, last, written, + (one ? "line " + first : "lines " + first + "-" + last) + ": written as the refusals said"); + } + Matcher lead = LEADING_LINE.matcher(error); + int at = error.lastIndexOf(LINE_FORM); + if (at < 0 || !lead.find()) { + return null; + } + int line = Integer.parseInt(lead.group(1)); + String text = error.substring(at + LINE_FORM.length()).strip(); + if (line < 1 || line > lines.size() || text.isEmpty() || text.contains("\n") + || !sameShape(lines.get(line - 1), text)) { + return null; + } + String old = lines.get(line - 1); + return new Fix( + line - 1, line, List.of(old.substring(0, LenientRepairs.indent(old)) + text), + "line " + line + ": written as the refusals said: " + text); + } + + /** + * The line to write is of the line it replaces: the same key, and a list item when that one is; a message that went + * on after the line to write would else give a line that is not the one meant. + */ + private static boolean sameShape(String old, String text) { + String o = old.strip(); + if (o.startsWith("- ") != text.startsWith("- ")) { + return false; + } + Matcher k = KEY.matcher(o.startsWith("- ") ? o.substring(2).stripLeading() : o); + if (!k.find()) { + return true; + } + Matcher t = KEY.matcher(text.startsWith("- ") ? text.substring(2).stripLeading() : text); + return t.find() && t.group(1).equals(k.group(1)); + } +} diff --git a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/AuthoringToolsRefusalFixesTest.java b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/AuthoringToolsRefusalFixesTest.java new file mode 100644 index 000000000000..fb4370cb61fe --- /dev/null +++ b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/AuthoringToolsRefusalFixesTest.java @@ -0,0 +1,139 @@ +/* + * 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.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.apache.camel.util.json.JsonArray; +import org.apache.camel.util.json.JsonObject; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * CAMEL-26758: the same refused content a third time is written with the fixes the refusals gave as exact text, when + * every problem has one and the result is valid. + */ +class AuthoringToolsRefusalFixesTest { + + private static final String FILE = "orders.camel.yaml"; + + /** r30n-5 contracts-rest-code-first step 1 (shortened): sent 15 times with the [] the refusal said to leave out. */ + private static final String BRACKETS = """ + - route: + id: get-order-by-id + from: + uri: direct:get-order-by-id + steps: + - setBody: + expression: + jsonpath: + expression: "$[?(@.orderId == '${header.id}')][]" + resultType: java.util.List + - log: "${body}" + """; + + private static final String FIXED = BRACKETS.replace("')][]\"", "')]\""); + + private static JsonObject write(Path dir, String content) { + return AuthoringTools.writeFile(new ToolContext(), dir, FILE, content, true); + } + + @Test + void theThirdSameWriteGetsTheFixTheRefusalsGave(@TempDir Path dir) throws IOException { + JsonObject first = write(dir, BRACKETS); + assertThat(first.getString("status")).as(String.valueOf(first)).isEqualTo("invalid"); + assertThat((JsonArray) first.get("errors")).singleElement().asString() + .endsWith("write the line as expression: \"$[?(@.orderId == '${header.id}')]\""); + // the second time the model still chooses its own fix + JsonObject second = write(dir, BRACKETS); + assertThat(second.getString("status")).isEqualTo("invalid"); + assertThat(second.getInteger("repeated")).isEqualTo(2); + assertThat(dir.resolve(FILE)).doesNotExist(); + + JsonObject third = write(dir, BRACKETS); + assertThat(third.getString("status")).as(String.valueOf(third)).isEqualTo("created"); + assertThat((JsonArray) third.get("repaired")).containsExactly( + "line 9: written as the refusals said: expression: \"$[?(@.orderId == '${header.id}')]\""); + assertThat(third.getString("message")).startsWith("Written with these repairs of the YAML") + .contains("The same content came a third time, so the write made the fixes the refusals gave"); + assertThat(Files.readString(dir.resolve(FILE))).isEqualTo(FIXED); + + // the count starts again: the same content once more is a first refusal + JsonObject again = write(dir, BRACKETS); + assertThat(again.getString("status")).isEqualTo("invalid"); + assertThat(again.get("repeated")).isNull(); + } + + @Test + void nothingIsWrittenWhenAProblemHasNoTextToWrite(@TempDir Path dir) { + // an option the log EIP does not have: the refusal says so, but gives no line to write + String content = BRACKETS.replace(" - log: \"${body}\"", " - log:\n message: \"${body}\"\n" + + " colour: red"); + JsonObject result = null; + for (int i = 1; i <= 4; i++) { + result = write(dir, content); + assertThat(result.getString("status")).as(String.valueOf(result)).isEqualTo("invalid"); + } + assertThat(result.getInteger("repeated")).isEqualTo(4); + assertThat(String.valueOf(result.get("errors"))).contains("colour"); + assertThat(dir.resolve(FILE)).doesNotExist(); + } + + @Test + void theThirdSameEditGetsTheFixTheRefusalsGave(@TempDir Path dir) throws IOException { + assertThat(write(dir, FIXED).getString("status")).isEqualTo("created"); + String find = "')]\"\n resultType: java.util.List"; + String replace = "')][]\"\n resultType: java.util.Map"; + for (int i = 1; i <= 2; i++) { + JsonObject refused = AuthoringTools.editFile(new ToolContext(), dir, FILE, find, replace); + assertThat(refused.getString("status")).as(String.valueOf(refused)).isEqualTo("invalid"); + } + JsonObject third = AuthoringTools.editFile(new ToolContext(), dir, FILE, find, replace); + assertThat(third.getString("status")).as(String.valueOf(third)).isEqualTo("edited"); + assertThat((JsonArray) third.get("repaired")).singleElement().asString() + .startsWith("line 9: written as the refusals said"); + assertThat(Files.readString(dir.resolve(FILE))).isEqualTo(FIXED.replace("java.util.List", "java.util.Map")); + } + + @Test + void anUnclosedQuoteIsWrittenAsTheRefusalsSaid(@TempDir Path dir) throws IOException { + // r28l-4, r29l-3 and r30l-2/5/6 route-filter-and-multicast step 1 sent this line 3 times each + String content = """ + - route: + from: + uri: direct:orders + steps: + - filter: + simple: "${body[status]} == 'paid' + steps: + - to: log:paid + """; + for (int i = 1; i <= 2; i++) { + JsonObject refused = write(dir, content); + assertThat(refused.getString("status")).as(String.valueOf(refused)).isEqualTo("invalid"); + assertThat((JsonArray) refused.get("errors")).anySatisfy(e -> assertThat(String.valueOf(e)) + .endsWith("write the line as simple: \"${body[status]} == 'paid'\"")); + } + JsonObject third = write(dir, content); + assertThat(third.getString("status")).as(String.valueOf(third)).isEqualTo("created"); + assertThat(Files.readString(dir.resolve(FILE))).isEqualTo(content.replace("== 'paid'\n", "== 'paid'\"\n")); + } +} diff --git a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/JsonPathChecksTest.java b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/JsonPathChecksTest.java index 45e08aa530e4..b9c56db2dacd 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/JsonPathChecksTest.java +++ b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/JsonPathChecksTest.java @@ -268,13 +268,16 @@ class JsonPathChecksTest { assertThat(JsonPathChecks.validateYamlJsonPath(yaml, catalog)).singleElement().isEqualTo( "Line 9: jsonpath $[?(@.orderId == '${header.id}')][] does not compile: Could not parse token starting at" + " position 33 (at []). Expected ?, ', 0-9, *; JsonPath has no [], and a filter" - + " or [*] gives a list already: write $[?(@.orderId == '${header.id}')]"); + + " or [*] gives a list already: write the line as expression: \"$[?(@.orderId ==" + + " '${header.id}')]\""); // without a placeholder, and in a split assertThat(JsonPathChecks.validateYamlJsonPath(yaml.replace("$[?(@.orderId == '${header.id}')][]", "$.lines[]"), catalog)).singleElement().asString() .startsWith("Line 9: jsonpath $.lines[] does not compile: Could not parse token starting at position 7" + " (at [])") - .endsWith("write $.lines"); + .endsWith("write the line as expression: \"$.lines\""); + // without the line (the expression tool), the expression alone + assertThat(JsonPathChecks.compileProblem(catalog, "$.lines[]", false)).endsWith("write $.lines"); assertThat(JsonPathChecks.validateYamlJsonPath(""" - route: from: diff --git a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/RefusalFixesTest.java b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/RefusalFixesTest.java new file mode 100644 index 000000000000..438924214fb8 --- /dev/null +++ b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/RefusalFixesTest.java @@ -0,0 +1,113 @@ +/* + * 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.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** CAMEL-26758: the two forms of exact text in a refusal, and when none is made. */ +class RefusalFixesTest { + + private static final String CONTENT = """ + - route: + from: + uri: direct:a + steps: + - filter: + simple: "${body} == 'a' + - to: log:x"""; + + @Test + void theLineFormKeepsTheIndentation() { + LenientRepairs.Result r = RefusalFixes.apply(CONTENT, List.of( + "line 6: the value opens a double quote and never closes it (a single quote inside a double-quoted value" + + " needs no escape); write the line as simple: \"${body} == 'a'\"")); + assertThat(r).isNotNull(); + assertThat(r.content()).isEqualTo(CONTENT.replace("== 'a'\n", "== 'a'\"\n")); + assertThat(r.applied()).containsExactly("line 6: written as the refusals said: simple: \"${body} == 'a'\""); + assertThat(r.moved()).isFalse(); + } + + @Test + void theBlockFormWritesTheLines() { + LenientRepairs.Result r = RefusalFixes.apply(CONTENT, List.of( + "line 5: something to fix. Write lines 5-6 as:\n - filter:\n simple: \"${body} == 'a'\"")); + assertThat(r).isNotNull(); + assertThat(r.content()).isEqualTo(CONTENT.replace("== 'a'\n", "== 'a'\"\n")); + assertThat(r.applied()).containsExactly("lines 5-6: written as the refusals said"); + } + + @Test + void oneLineWrittenAsSeveral() { + LenientRepairs.Result r = RefusalFixes.apply("a:\n b: 1\\n c: 2\nd: 3", List.of( + "line 2: the value of b is followed by \\n. Write line 2 as these 2 lines:\n b: 1\n c: 2")); + assertThat(r).isNotNull(); + assertThat(r.content()).isEqualTo("a:\n b: 1\n c: 2\nd: 3"); + assertThat(r.applied()).containsExactly("line 2: written as the refusals said"); + assertThat(r.moved()).isTrue(); + } + + @Test + void aProblemWithoutTextIsLeftToTheCheckThatFollows() { + // the fix of line 6 is made; whether the problem of line 7 is gone, the write's check says + LenientRepairs.Result r = RefusalFixes.apply(CONTENT, List.of( + "line 6: the value opens a double quote and never closes it; write the line as simple: \"${body} == 'a'\"", + "Line 7: to: property 'foo' is not defined in the schema")); + assertThat(r).isNotNull(); + assertThat(r.content()).isEqualTo(CONTENT.replace("== 'a'\n", "== 'a'\"\n")); + assertThat(r.applied()).hasSize(1); + // no problem with text: nothing to make + assertThat(RefusalFixes.apply(CONTENT, List.of("Line 7: to: property 'foo' is not defined in the schema"))) + .isNull(); + assertThat(RefusalFixes.apply(CONTENT, List.of())).isNull(); + } + + @Test + void theLineToWriteIsOfTheLineItReplaces() { + // another key, or a list item written as a key: a message that went on after the line to write + assertThat(RefusalFixes.apply(CONTENT, List.of("line 6: x; write the line as message: \"${body}\""))).isNull(); + assertThat(RefusalFixes.apply(CONTENT, List.of("line 7: x; write the line as to: log:y"))).isNull(); + assertThat(RefusalFixes.apply(CONTENT, List.of("line 6: x; write the line as simple: \"a\" (see notes)\nmore"))) + .isNull(); + assertThat(RefusalFixes.apply(CONTENT, List.of("line 9: x; write the line as simple: \"a\""))).isNull(); + } + + @Test + void twoFixesOfTheSameLinesAreNotMade() { + assertThat(RefusalFixes.apply(CONTENT, List.of( + "line 5: x. Write lines 5-6 as:\n - filter:\n simple: \"a\"", + "line 6: y; write the line as simple: \"b\""))).isNull(); + // the same fix twice is made once + String error = "line 6: y; write the line as simple: \"b\""; + LenientRepairs.Result r = RefusalFixes.apply(CONTENT, List.of(error, error)); + assertThat(r).isNotNull(); + assertThat(r.applied()).hasSize(1); + } + + @Test + void aBlockShownInPartIsNotWritten() { + assertThat(RefusalFixes.apply(CONTENT, List.of( + "line 4: x. Write lines 4-6 as:\n steps:\n - filter:\n(and the 1 lines after them, two spaces" + + " further right too)"))) + .isNull(); + // a count that does not fit the lines + assertThat(RefusalFixes.apply(CONTENT, List.of("line 5: x. Write lines 5-6 as:\n - filter:"))).isNull(); + } +} diff --git a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java index 6be9030b486c..476a6772178a 100644 --- a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java +++ b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java @@ -594,12 +594,13 @@ public class YamlValidator { String value = colon >= 0 ? line.substring(colon + 1).trim() : line.trim(); long quotes = value.chars().filter(c -> c == '"').count() - value.split("\\\\\"", -1).length + 1; if (value.startsWith("\"") && quotes % 2 == 1) { - // the line to write, not a description of the edit: a model copies the line (CAMEL-24906) + // the line to write, not a description of the edit: a model copies the line (CAMEL-24906); it ends the + // message, so the write tool can apply it when the same lines come a third time (CAMEL-26758) String indent = line.substring(0, line.length() - line.stripLeading().length()); String key = colon >= 0 ? line.stripLeading().substring(0, line.stripLeading().indexOf(':') + 1) : ""; - return hint("line " + start.line() + ": the value opens a double quote and never closes it; write the line" - + " as " + (indent + key + " " + value + "\"").strip() - + " (a single quote inside a double-quoted value needs no escape)"); + return hint("line " + start.line() + ": the value opens a double quote and never closes it (a single quote" + + " inside a double-quoted value needs no escape); write the line as " + + (indent + key + " " + value + "\"").strip()); } return null; } 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 896e5f6171b5..fc5aeb0052c5 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 @@ -439,9 +439,10 @@ public class YamlValidatorPropertyHintTest { resultType: java.util.List - log: "done" """); + // the line to write ends the message (CAMEL-26758: the write tool applies it on the third same write) assertThat(errors).extracting(Error::getMessage) .anyMatch(m -> m.startsWith("line 8: the value opens a double quote and never closes it") - && m.contains("write the line as expression: \"$[?(@.sku == '${header.sku}')]\"")); + && m.endsWith("write the line as expression: \"$[?(@.sku == '${header.sku}')]\"")); } /** CAMEL-24906: the value corrected twice ends with two quotes, and the line to write says so. */
