This is an automated email from the ASF dual-hosted git repository.
davsclaus pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/main by this push:
new 18d98ea5f7f1 CAMEL-26410: camel-jbang - AI tools: a refused write
reports the problems behind a YAML indentation the write repairs by itself
(#28717)
18d98ea5f7f1 is described below
commit 18d98ea5f7f121f4d9619b39933826b5939a5fad
Author: Claus Ibsen <[email protected]>
AuthorDate: Fri Oct 30 10:46:35 2026 +0100
CAMEL-26410: camel-jbang - AI tools: a refused write reports the problems
behind a YAML indentation the write repairs by itself (#28717)
The write and edit tools repair a YAML shape with one reading, such as a
list
item two columns off. When the repaired content had another problem, the
repair was dropped and the write was refused with the indentation error
only:
the parser stops there, so the other checks never ran. A local model then
sent
the same lines up to 19 times in a row.
Now the write is still refused, but with the problems of the repaired
content,
and a note that the write repairs the shape by itself. When a problem of the
content as written is still there, or a repair moved lines, the answer stays
as before.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
---
.../dsl/jbang/core/commands/ai/AuthoringTools.java | 56 +++++++++-
.../dsl/jbang/core/commands/ai/LenientRepairs.java | 11 +-
.../commands/ai/AuthoringToolsLenientTest.java | 121 +++++++++++++++++++++
3 files changed, 179 insertions(+), 9 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 cbbb5be0abae..fd4ce3e0dd1a 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
@@ -35,6 +35,7 @@ import java.util.Collection;
import java.util.Comparator;
import java.util.EnumSet;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Locale;
@@ -748,6 +749,10 @@ public final class AuthoringTools {
result.put("message", "The file was not changed: the result has
validation errors. Fix them and call"
+ " camel_edit_file again.");
}
+ if ("invalid".equals(result.getString("status")) &&
result.get("repairable") instanceof JsonArray repairable) {
+ result.put("message", result.getString("message") + " "
+ +
repairableNote(repairable.stream().map(String::valueOf).toList()));
+ }
return result;
}
@@ -1672,6 +1677,7 @@ public final class AuthoringTools {
}
List<String> problemsBefore = List.of();
List<String> repaired = List.of();
+ List<String> repairable = List.of();
if (validate && SourceValidator.isValidatableFile(file)) {
// a missing consumer of a direct: endpoint does not refuse the
write: it is often a file not written yet
List<String> errors = SourceValidator.validate(file, content,
ctx.catalog(), ctx.propertyLineValidator(), dir,
@@ -1690,11 +1696,16 @@ public final class AuthoringTools {
// CAMEL-25532: the shapes with one reading are repaired, and
the write goes ahead when that leaves no
// new problem; the answer says what was repaired
Repaired r = repair(ctx, dir, file, content, errors, before,
exists);
- if (r != null) {
+ List<String> hidden = hiddenProblems(r, errors);
+ if (r != null && r.left().isEmpty()) {
content = r.content();
repaired = r.applied();
problemsBefore = r.stillThere();
errors = List.of();
+ } else if (hidden != null) {
+ // the write repairs the shape itself: the problems to fix
are the ones the shape hid
+ errors = hidden;
+ repairable = r.applied();
}
}
if (!errors.isEmpty()) {
@@ -1714,6 +1725,10 @@ public final class AuthoringTools {
+ " it, refused " + times + " times
now for the same problems. Change what"
+ " the errors say; where an error
shows the lines to write, copy them.");
}
+ if (!repairable.isEmpty()) {
+ result.put("repairable", new JsonArray(repairable));
+ result.put("message", result.getString("message") + " " +
repairableNote(repairable));
+ }
putKameletGuide(result, file, errors);
return result;
}
@@ -1884,12 +1899,17 @@ public final class AuthoringTools {
/** How many times the repairs are applied and the content validated again
(a repair can reveal the next one). */
private static final int REPAIR_ROUNDS = 8;
- private record Repaired(String content, List<String> applied, List<String>
stillThere) {
+ /**
+ * The content with the shapes that have one reading repaired; left has
the problems it still has (none: the write
+ * goes ahead with the repairs), and moved says an entry moved to other
lines.
+ */
+ private record Repaired(
+ String content, List<String> applied, List<String> stillThere,
List<String> left, boolean moved) {
}
/**
- * The content with the shapes that have one reading repaired, when that
leaves no problem the file did not have;
- * null otherwise (the write is refused as it was, with the problems of
the content as written).
+ * The content with the shapes that have one reading repaired, with the
problems it still has once repaired; null
+ * when nothing could be repaired (the write is refused with the problems
of the content as written).
*/
private static Repaired repair(
ToolContext ctx, Path dir, String file, String content,
List<String> errors, List<String> before,
@@ -1898,20 +1918,44 @@ public final class AuthoringTools {
List<String> applied = new ArrayList<>();
List<String> left = errors;
List<String> stillThere = new ArrayList<>();
+ boolean moved = false;
for (int round = 0; round < REPAIR_ROUNDS && !left.isEmpty(); round++)
{
LenientRepairs.Result r = LenientRepairs.repair(current, left);
if (r == null) {
- return null;
+ break;
}
current = r.content();
applied.addAll(r.applied());
+ moved |= r.moved();
left = SourceValidator.validate(file, current, ctx.catalog(),
ctx.propertyLineValidator(), dir, null, false);
stillThere = new ArrayList<>();
if (!left.isEmpty() && exists) {
left = newProblems(before, left, stillThere);
}
}
- return left.isEmpty() ? new Repaired(current, applied, stillThere) :
null;
+ return applied.isEmpty() ? null : new Repaired(current, applied,
stillThere, left, 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
+ * r24 of the benchmark), while with the column repaired the validator had
the real problem: a steps: at the route
+ * level, a field read on a variable that holds a file. Null when a
problem of the content as written is still there
+ * (the write cannot repair it), or the repairs moved lines, so the line
numbers would not be the model's.
+ */
+ private static List<String> hiddenProblems(Repaired r, List<String>
errors) {
+ if (r == null || r.left().isEmpty() || r.moved()) {
+ return null;
+ }
+ Set<String> written = new HashSet<>();
+ errors.forEach(e -> written.add(withoutLine(e)));
+ return r.left().stream().anyMatch(e ->
written.contains(withoutLine(e))) ? null : r.left();
+ }
+
+ /** What the write repairs by itself, said with the problems it still
refuses. */
+ static String repairableNote(List<String> repairable) {
+ return "The YAML shape needs no change from you: the write repairs it
by itself (" + String.join("; ", repairable)
+ + "). The errors are what the content has with that repaired.";
}
private static final Pattern UNCONSUMED_DIRECT = Pattern.compile("sends to
(direct:[^,\\s]+), and no route consumes it");
diff --git
a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/LenientRepairs.java
b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/LenientRepairs.java
index 631b162a175f..dfc653aaef76 100644
---
a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/LenientRepairs.java
+++
b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/LenientRepairs.java
@@ -36,8 +36,11 @@ import java.util.regex.Pattern;
*/
final class LenientRepairs {
- /** The repaired content and what was done, one line each ("line 13:
steps: indented under split:"). */
- record Result(String content, List<String> applied) {
+ /**
+ * The repaired content and what was done, one line each ("line 13: steps:
indented under split:"); moved when an
+ * entry moved to other lines, so the line numbers of the content are not
the ones the model wrote.
+ */
+ record Result(String content, List<String> applied, boolean moved) {
}
/** A repair of one place of the content; a move changes the lines above
it, so it goes alone. */
@@ -125,12 +128,14 @@ final class LenientRepairs {
todo =
todo.stream().sorted(Comparator.comparingInt(Repair::line).reversed()).toList();
Set<Integer> done = new HashSet<>();
List<String> applied = new ArrayList<>();
+ boolean moved = false;
for (Repair r : todo) {
if (done.add(r.line()) && r.action().apply(lines)) {
applied.add(0, r.description());
+ moved |= r.move();
}
}
- return applied.isEmpty() ? null : new Result(String.join("\n", lines),
applied);
+ return applied.isEmpty() ? null : new Result(String.join("\n", lines),
applied, moved);
}
private static List<Repair> repairsFor(String error, List<String> lines) {
diff --git
a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/AuthoringToolsLenientTest.java
b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/AuthoringToolsLenientTest.java
index 7ed31de5a0b3..190087096e95 100644
---
a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/AuthoringToolsLenientTest.java
+++
b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/AuthoringToolsLenientTest.java
@@ -258,6 +258,127 @@ class AuthoringToolsLenientTest {
""");
assertThat(result.getString("status")).isEqualTo("invalid");
assertThat(result.get("repaired")).isNull();
+ assertThat(result.get("repairable")).isNull();
+ }
+
+ @Test
+ void aListItemTwoColumnsOffDoesNotHideTheProblemBehindIt(@TempDir Path
dir) throws IOException {
+ // r23l-3, r24l-2 and r24l-6 route-aggregator step 1: the log two
columns off stopped the parser, so the steps:
+ // at the route level was never reported, and the column alone was
refused up to 19 times in a row
+ String content = """
+ - route:
+ id: shipments
+ from:
+ uri: direct:shipment
+ steps:
+ - aggregate:
+ aggregationStrategy:
"#class:org.apache.camel.processor.aggregate.GroupedBodyAggregationStrategy"
+ correlationExpression:
+ simple:
+ expression: "${header.orderId}"
+ completionSize: 2
+ - log:
+ message: "Shipment complete"
+ """;
+ JsonObject result = write(dir, content);
+
assertThat(result.getString("status")).as(String.valueOf(result)).isEqualTo("invalid");
+ assertThat(result.get("errors").toString()).contains("steps: goes
under from:")
+ .doesNotContain("every item of a list");
+ assertThat(result.get("repairable").toString()).contains("the list
item moved to the column of the list");
+ assertThat(result.getString("message")).contains("the write repairs it
by itself");
+ assertThat(result.get("repaired")).isNull();
+
+ // the steps: under from:, the log still two columns off: the write
goes ahead with the repair
+ JsonObject again = write(dir, """
+ - route:
+ id: shipments
+ from:
+ uri: direct:shipment
+ steps:
+ - aggregate:
+ aggregationStrategy:
"#class:org.apache.camel.processor.aggregate.GroupedBodyAggregationStrategy"
+ correlationExpression:
+ simple:
+ expression: "${header.orderId}"
+ completionSize: 2
+ - log:
+ message: "Shipment complete"
+ """);
+ assertRepairedAndValid(dir, again, "the list item moved to the column
of the list");
+ }
+
+ @Test
+ void aListItemTwoColumnsOffDoesNotHideABodyTypeProblem(@TempDir Path dir) {
+ // r24n-2 route-enrich-and-dedupe step 2: the log after a pollEnrich
two columns off, refused 16 times in a row,
+ // hid that the log reads a field of the file that the pollEnrich put
in a variable
+ JsonObject result = write(dir, """
+ - route:
+ from:
+ uri: file:orders
+ steps:
+ - unmarshal:
+ json: {}
+ - pollEnrich:
+ expression:
+ simple:
+ expression:
"file:customers?fileName=${body[customer]}.json&noop=true"
+ timeout: 5000
+ variableReceive: customerData
+ - log:
+ message: "Order for
${variable.customerData[name]}"
+ """);
+
assertThat(result.getString("status")).as(String.valueOf(result)).isEqualTo("invalid");
+ assertThat(result.get("errors").toString()).contains("holds the file
as it was read")
+ .doesNotContain("every item of a list");
+ assertThat(result.get("repairable").toString()).contains("the list
item moved to the column of the list");
+ }
+
+ @Test
+ void anEditKeepsWhatTheWriteRepairsByItself(@TempDir Path dir) throws
IOException {
+ Files.writeString(dir.resolve(FILE), """
+ - route:
+ from:
+ uri: file:orders
+ steps:
+ - unmarshal:
+ json: {}
+ - pollEnrich:
+ expression:
+ simple:
+ expression:
"file:customers?fileName=${body[customer]}.json&noop=true"
+ timeout: 5000
+ variableReceive: customerData
+ """);
+ JsonObject result = AuthoringTools.editFile(new ToolContext(), dir,
FILE, """
+ variableReceive: customerData
+ """, """
+ variableReceive: customerData
+ - log:
+ message: "Order for
${variable.customerData[name]}"
+ """);
+
assertThat(result.getString("status")).as(String.valueOf(result)).isEqualTo("invalid");
+ assertThat(result.get("errors").toString()).contains("holds the file
as it was read");
+ assertThat(result.getString("message")).startsWith("The file was not
changed")
+ .contains("the write repairs it by itself");
+ }
+
+ @Test
+ void aProblemTheWriteCannotRepairKeepsTheAnswerAsItWas(@TempDir Path dir) {
+ // the case of library: is repaired, the misspelt key is not: both are
said as written, nothing is hidden
+ JsonObject result = write(dir, """
+ - route:
+ from:
+ uri: timer:x
+ steps:
+ - unmarshal:
+ json:
+ library: jackson
+ - log:
+ mesage: "x"
+ """);
+ assertThat(result.getString("status")).isEqualTo("invalid");
+ assertThat(result.get("repairable")).isNull();
+ assertThat(result.get("errors").toString()).contains("mesage");
}
@Test
