davsclaus commented on code in PR #27422:
URL: https://github.com/apache/camel/pull/27422#discussion_r4193926983
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/AuthoringTools.java:
##########
@@ -160,6 +160,10 @@ static void register(Consumer<ToolDescriptor> registry) {
.param("content", "string", "The source to validate", false)
.param("camelVersion", "string", VERSION_DESC, false)
.core(true)
+ // the same content gets the same answer: a model that
validates it again and again is stuck (CAMEL-25371)
+ .deterministicWhen("content")
Review Comment:
Agreed, good find. Fixed in 0f284acb0b: `deterministicWhen("content",
"directory")` now makes a call deterministic only when `content` is given and
`directory` is not, as the checks read the sibling files only with a directory.
`validatingContentAgainstADirectoryIsNotARepeat` pins it: four identical calls
with a directory are all answered in full.
_Claude Code on behalf of davsclaus_
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/RepeatedToolCalls.java:
##########
@@ -51,7 +51,7 @@ public final class RepeatedToolCalls {
* identical calls.
*/
public synchronized JsonObject repeatOf(ToolDescriptor tool, Map<String,
?> args) {
- if (tool == null || !tool.isDeterministic()) {
+ if (tool == null || !tool.isDeterministic(args)) {
Review Comment:
Fixed in 0f284acb0b: the argument that makes the call deterministic
(`content`) is keyed by a SHA-256 of its exact text, so a whitespace-only fix
such as removing the newline before `<?xml` is validated, and the key no longer
holds the source. Short catalog arguments are still trimmed. Covered by
`contentThatOnlyDiffersInWhitespaceIsAnotherQuestion`.
_Claude Code on behalf of davsclaus_
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/AuthoringTools.java:
##########
@@ -174,8 +178,16 @@ static void register(Consumer<ToolDescriptor> registry) {
throw new ToolExecutionException("No such file in
the directory: " + file);
}
content = read(path);
+ return validate(ctx, dir, file, content).toJson();
}
- return validate(ctx, dir, file, content).toJson();
+ JsonObject result = validate(ctx, dir, file, content);
+ if (sameAsOnDisk(ctx, directory, file, content)) {
+ // a model that meant to change the file validates the
old version: say so (CAMEL-25371)
+ result.put("sameAsFile", true);
+ result.put("note", "This content is the same as " +
file + " on disk: an edit you meant to make"
Review Comment:
Good point about the camel_write_file and camel_get_files flows. Changed in
0f284acb0b to your conditional wording: "This content is the same as <file> on
disk: if you meant to change the file, that change is not in this content."
_Claude Code on behalf of davsclaus_
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/ToolDescriptor.java:
##########
@@ -128,6 +145,21 @@ public boolean isDeterministic() {
return deterministic;
}
+ /**
+ * Whether this call gives the same answer each time: the tool is
deterministic, or the argument that makes it so.
+ */
+ public boolean isDeterministic(java.util.Map<String, ?> args) {
Review Comment:
Fixed in 0f284acb0b: `Map` is imported, and the new `AuthoringToolsTest`
case uses the imported `TempDir`, `Path` and `Files`.
_Claude Code on behalf of davsclaus_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
