This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch fix/CAMEL-25556 in repository https://gitbox.apache.org/repos/asf/camel.git
commit 1b522ae3539161d57ef16a0bad411ba66a97f645 Author: Claus Ibsen <[email protected]> AuthorDate: Sun Oct 11 13:28:50 2026 +0200 CAMEL-25556: camel-jbang-mcp - a write the same as the last refused one says so, and the hint for an option at the column of its EIP shows the lines to write A local model that wrote steps: at the column of - split: sent the same content sixteen times in a row after the hint of CAMEL-25529, until it ran out of tool calls: it does not see the problem in its own lines, and copies lines it is given. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01STT6whBgK1AqsSsUKrnE8m --- .../dsl/jbang/core/commands/ai/AuthoringTools.java | 29 +++++ .../dsl/jbang/core/commands/ai/IndentLines.java | 126 +++++++++++++++++++++ .../jbang/core/commands/ai/SourceValidator.java | 3 + .../core/commands/ai/AuthoringToolsLoopTest.java | 117 +++++++++++++++++++ 4 files changed, 275 insertions(+) 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 61e105595dc7..4d2be4bff8a0 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.List; import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Consumer; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -638,6 +639,10 @@ public final class AuthoringTools { result.put("edits", edit.get("edits")); result.put("editedAtLines", edit.get("editedAtLines")); } + } else if (result.get("repeated") != null) { + result.put("message", "The file was not changed: the edit gives the same content as your last attempt," + + " refused " + result.getInteger("repeated") + " times now for the same problems." + + " Change what the errors say; where an error shows the lines to write, copy them."); } else { result.put("message", "The file was not changed: the result has validation errors. Fix them and call" + " camel_edit_file again."); @@ -1093,6 +1098,16 @@ 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."); + 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 + // (CAMEL-25556) + result.put("repeated", times); + result.put("message", "The file was not written: the content is the same as your last write of" + + " it, refused " + times + " times now for the same problems. Change what" + + " the errors say; where an error shows the lines to write, copy them."); + } putKameletGuide(result, file, errors); return result; } @@ -1127,6 +1142,7 @@ public final class AuthoringTools { } catch (IOException e) { throw new ToolExecutionException("Failed to write " + path + ": " + e.getMessage()); } + LAST_REFUSED.remove(path.toAbsolutePath().normalize()); JsonObject result = new JsonObject(); result.put("status", exists ? "overwritten" : "created"); result.put("file", file); @@ -1180,6 +1196,19 @@ public final class AuthoringTools { return result; } + /** The content of the last refused write of each file, and how many times in a row it was refused. */ + private record Refused(String content, int times) { + } + + private static final Map<Path, Refused> LAST_REFUSED = new ConcurrentHashMap<>(); + + /** How many times in a row this content of the file has been refused, this time included. */ + static int refusedAgain(Path path, String content) { + return LAST_REFUSED.merge(path.toAbsolutePath().normalize(), new Refused(content, 1), + (last, now) -> last.content().equals(content) ? new Refused(content, last.times() + 1) : now) + .times(); + } + 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/IndentLines.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/IndentLines.java new file mode 100644 index 000000000000..91ca88904931 --- /dev/null +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/IndentLines.java @@ -0,0 +1,126 @@ +/* + * 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.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * The lines to write for an option of a step's EIP written at the column of the EIP (steps: next to - split:): the + * validator says which way to indent them (CAMEL-25529), and a model that does not see it in its own lines writes the + * same YAML again (sixteen times in a row in the local-model benchmark); the lines as they must be, it copies + * (CAMEL-25556). + */ +final class IndentLines { + + /** The validator's message for it: "Line 13: must have at most 1 properties (steps: lines up with split:, ...". */ + private static final Pattern OPTION_AT_EIP = Pattern.compile( + "^Line (\\d+): must have at most 1 properties \\((\\w+): lines up with (\\w+):, so it is read as a second key"); + + /** How many lines are shown at most; a longer block is said in words after them. */ + private static final int MAX_LINES = 12; + + private IndentLines() { + } + + /** The messages, with the lines to write after each message of an option at the column of its EIP. */ + static List<String> withLines(List<String> messages, String content) { + if (messages.isEmpty() || content == null) { + return messages; + } + String[] lines = content.split("\n", -1); + List<String> answer = new ArrayList<>(messages.size()); + for (String message : messages) { + Matcher m = OPTION_AT_EIP.matcher(message); + String written = m.find() ? linesToWrite(lines, Integer.parseInt(m.group(1)) - 1, m.group(2)) : null; + answer.add(written != null ? message + written : message); + } + return answer; + } + + /** " Write lines 15-17 as:" and the lines two spaces further right; null when the key is not found. */ + static String linesToWrite(String[] lines, int itemIndex, String key) { + if (itemIndex < 0 || itemIndex >= lines.length) { + return null; + } + String item = lines[itemIndex]; + int dash = indent(item); + if (!item.startsWith("- ", dash)) { + return null; + } + int keyColumn = dash + 2; + for (int j = itemIndex + 1; j < lines.length; j++) { + String l = lines[j]; + if (l.isBlank()) { + continue; + } + int i = indent(l); + if (i < keyColumn) { + return null; // past the item + } + if (i == keyColumn && l.startsWith(key + ":", i)) { + int end = blockEnd(lines, j); + StringBuilder sb = new StringBuilder(" Write lines ").append(j + 1).append('-').append(end) + .append(" as:"); + int shown = 0; + for (int k = j; k < end; k++) { + if (shown == MAX_LINES) { + sb.append("\n(and the ").append(end - k).append(" lines after them, two spaces further right too)"); + break; + } + sb.append('\n').append(lines[k].isBlank() ? "" : " " + lines[k]); + shown++; + } + return sb.toString(); + } + } + return null; + } + + /** + * The end (exclusive) of the block of a key: the lines below it that are more indented, and the items of a list at + * the key's own column (YAML allows them there), without the blank lines at its end. + */ + private static int blockEnd(String[] lines, int start) { + int col = indent(lines[start]); + boolean keyWithoutValue = lines[start].strip().endsWith(":"); + int end = start + 1; + while (end < lines.length) { + String l = lines[end]; + if (l.isBlank() || indent(l) > col + || keyWithoutValue && indent(l) == col && l.startsWith("- ", col)) { + end++; + } else { + break; + } + } + while (end > start + 1 && lines[end - 1].isBlank()) { + end--; + } + return end; + } + + private static int indent(String line) { + int i = 0; + while (i < line.length() && line.charAt(i) == ' ') { + i++; + } + return i; + } +} diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/SourceValidator.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/SourceValidator.java index 2de9e60532c1..08a9d45a4a97 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/SourceValidator.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/SourceValidator.java @@ -150,6 +150,9 @@ public final class SourceValidator { // a header read where the route keeps the name in an exchange property, or the reverse (CAMEL-25516) msgs = new ArrayList<>(HeaderPropertyMixups.validate(content)); } + // an option at the column of its EIP: the lines to write, which a model copies (the words it does not, + // CAMEL-25556) + msgs = IndentLines.withLines(msgs, content); return KameletChecks.withTemplateHints(name, content, msgs); } if (name.endsWith(".properties")) { diff --git a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/AuthoringToolsLoopTest.java b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/AuthoringToolsLoopTest.java new file mode 100644 index 000000000000..7e215063cfa7 --- /dev/null +++ b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/AuthoringToolsLoopTest.java @@ -0,0 +1,117 @@ +/* + * 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 java.util.List; + +import org.apache.camel.catalog.DefaultCamelCatalog; +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; + +/** + * A model that does not see the problem in its own lines writes them again, as it was told: the lines to write for an + * option at the column of its EIP, and a write the same as the last refused one says so. + */ +class AuthoringToolsLoopTest { + + private static final String FILE = "demo.camel.yaml"; + + // as the local model wrote it sixteen times in a row: steps: at the column of split: + private static final String STEPS_NEXT_TO_SPLIT = """ + - route: + from: + uri: timer:orders + steps: + - split: + expression: + simple: "${body}" + steps: + - log: + message: "pick ${body}" + """; + + @Test + void theHintShowsTheLinesToWrite() { + List<String> errors = SourceValidator.validate(FILE, STEPS_NEXT_TO_SPLIT, new DefaultCamelCatalog(), null); + assertThat(errors).singleElement().satisfies(m -> assertThat(m) + .contains("steps: lines up with split:") + .endsWith(" Write lines 8-10 as:\n" + + " steps:\n" + + " - log:\n" + + " message: \"pick ${body}\"")); + } + + @Test + void theSameRefusedContentAgainIsSaidToBeTheSame(@TempDir Path dir) throws IOException { + JsonObject first = write(dir, STEPS_NEXT_TO_SPLIT); + assertThat(first.getString("status")).isEqualTo("invalid"); + assertThat(first.get("repeated")).isNull(); + + JsonObject second = write(dir, STEPS_NEXT_TO_SPLIT); + assertThat(second.getInteger("repeated")).isEqualTo(2); + assertThat(second.getString("message")) + .contains("the same as your last write of it, refused 2 times now") + .contains("where an error shows the lines to write, copy them"); + assertThat(write(dir, STEPS_NEXT_TO_SPLIT).getInteger("repeated")).isEqualTo(3); + + // other content starts again, and a write that goes through forgets it + assertThat(write(dir, STEPS_NEXT_TO_SPLIT.replace("pick", "take")).get("repeated")).isNull(); + assertThat(write(dir, + STEPS_NEXT_TO_SPLIT.replace(" steps:\n - log:\n message: \"pick ${body}\"", + " steps:\n - log:\n message: \"pick ${body}\"")) + .getString("status")).isIn("created", "overwritten"); + assertThat(write(dir, STEPS_NEXT_TO_SPLIT).get("repeated")).isNull(); + } + + @Test + void theSameRefusedEditAgainIsSaidToBeTheSame(@TempDir Path dir) throws IOException { + Files.writeString(dir.resolve(FILE), """ + - route: + from: + uri: timer:orders + steps: + - log: + message: "order ${body}" + """); + String find = " - log:\n message: \"order ${body}\""; + String replace = """ + - split: + expression: + simple: "${body}" + steps: + - log: + message: "pick ${body}\""""; + assertThat(edit(dir, find, replace).getString("message")).startsWith("The file was not changed: the result"); + JsonObject again = edit(dir, find, replace); + assertThat(again.getInteger("repeated")).isEqualTo(2); + assertThat(again.getString("message")).contains("the edit gives the same content as your last attempt"); + } + + private static JsonObject write(Path dir, String content) { + return AuthoringTools.writeFile(new ToolContext(), dir, FILE, content, true); + } + + private static JsonObject edit(Path dir, String find, String replace) { + return AuthoringTools.editFile(new ToolContext(), dir, FILE, find, replace); + } +}
