davsclaus commented on code in PR #24594:
URL: https://github.com/apache/camel/pull/24594#discussion_r3565015591
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanel.java:
##########
@@ -323,14 +443,200 @@ boolean handleKeyEvent(KeyEvent ke) {
return true;
}
- private void submitQuestion() {
- String question = inputBuffer.toString().trim();
+ /**
+ * Completes the slash command name at the cursor. With multiple matches,
TAB first fills in the longest common
+ * prefix; once no further prefix can be added it cycles forward through
the matches (wrapping so every match is
+ * reachable with TAB alone). A single match is completed fully and a
trailing space is appended. Shift+TAB cycles
+ * backward. TAB is a no-op unless the buffer is a partial command name
(starts with {@code /}, no arguments yet).
+ */
+ private void handleTabCompletion(boolean backward) {
+ String text = inputBuffer.toString();
+ boolean continuing = text.equals(completionSnapshot) &&
completionMatches != null && completionMatches.size() > 1;
+ if (continuing) {
+ int size = completionMatches.size();
+ if (completionCycleIndex < 0) {
+ completionCycleIndex = backward ? size - 1 : 0;
+ } else {
+ completionCycleIndex = backward
+ ? (completionCycleIndex - 1 + size) % size
+ : (completionCycleIndex + 1) % size;
+ }
+ applyCompletionToken(completionMatches.get(completionCycleIndex),
false);
+ return;
+ }
+
+ List<String> names = slashCommands.completionsFor(text).stream()
+ .map(AiSlashCommandRegistry.Descriptor::name)
+ .toList();
+ if (names.isEmpty()) {
+ completionMatches = null;
+ completionCycleIndex = -1;
+ completionSnapshot = null;
+ return;
+ }
+ if (names.size() == 1) {
+ applyCompletionToken(names.get(0), true);
+ // A single completion ends with a trailing space, so there is
nothing left to cycle.
+ completionMatches = null;
+ completionCycleIndex = -1;
+ completionSnapshot = null;
+ return;
+ }
+ String currentToken = text.substring(1);
+ String prefix = longestCommonPrefix(names);
+ completionMatches = names;
+ if (prefix.length() > currentToken.length()) {
+ applyCompletionToken(prefix, false);
+ completionCycleIndex = -1;
+ } else {
+ completionCycleIndex = backward ? names.size() - 1 : 0;
+ applyCompletionToken(names.get(completionCycleIndex), false);
+ }
+ }
+
+ private void applyCompletionToken(String token, boolean trailingSpace) {
+ inputBuffer.setLength(0);
+ inputBuffer.append('/').append(token);
+ if (trailingSpace) {
+ inputBuffer.append(' ');
+ }
+ cursorPos = inputBuffer.length();
+ completionSnapshot = inputBuffer.toString();
+ }
+
+ private static String longestCommonPrefix(List<String> values) {
+ String prefix = values.get(0);
+ for (int i = 1; i < values.size() && !prefix.isEmpty(); i++) {
+ String value = values.get(i);
+ int max = Math.min(prefix.length(), value.length());
+ int j = 0;
+ while (j < max && prefix.charAt(j) == value.charAt(j)) {
+ j++;
+ }
+ prefix = prefix.substring(0, j);
+ }
+ return prefix;
+ }
+
+ private void submitInput() {
+ String input = inputBuffer.toString().trim();
inputBuffer.setLength(0);
cursorPos = 0;
scrollOffset = 0;
+ if (input.startsWith("/")) {
+ executeSlashCommand(input);
+ } else {
+ submitQuestion(input);
+ }
+ }
+
+ private void executeSlashCommand(String input) {
+ Optional<AiSlashCommandRegistry.ParsedCommand> parsed =
slashCommands.parse(input);
+ if (parsed.isPresent() && thinking.get()) {
+ String name = parsed.get().descriptor().name();
+ if ("provider".equals(name) || "model".equals(name)) {
+ conversation.add(new ConversationEntry(
+ AiRole.SYSTEM,
+ "Wait for the current operation to finish before
changing provider or model."));
+ return;
+ }
+ }
+
+ AiSlashCommandRegistry.CommandResult result =
slashCommands.execute(input, slashCommandContext);
+ if (result.cliRequest() != null) {
+ AiCliCommandExecutor.Request request = result.cliRequest();
+ conversation.add(new ConversationEntry(AiRole.SYSTEM, "Running " +
request.displayText()));
+ // Runs in the background without entering the panel's "thinking"
state, so the user can keep typing
+ // and asking questions while the command runs. Esc still cancels
it via interruptBusyOperation().
+ CompletableFuture<AiCliCommandExecutor.Result> future =
slashCommandContext.executeCli(request);
+ activeCliCommand = future;
+ future.whenComplete(this::handleCliCompletion);
+ return;
+ }
+ if (result.text() != null && !result.text().isBlank()) {
+ conversation.add(new ConversationEntry(result.role(),
result.text()));
+ }
+ }
+
+ private void handleCliCompletion(AiCliCommandExecutor.Result cliResult,
Throwable error) {
+ CompletableFuture<AiCliCommandExecutor.Result> future =
activeCliCommand;
+ synchronized (this) {
+ if (future == null || activeCliCommand != future) {
+ return;
+ }
+ activeCliCommand = null;
+ }
+ thinking.set(false);
+ thinkingVerb = null;
+
+ if (error != null) {
+ String displayText = cliResult != null ? cliResult.displayText() :
"command";
+ conversation.add(new ConversationEntry(
+ AiRole.ERROR, "Failed to run " + displayText + ": " +
error.getMessage()));
+ scrollOffset = 0;
+ return;
+ }
+
+ if (cliResult.exitCode() == 0) {
+ conversation.add(new ConversationEntry(
+ AiRole.SYSTEM,
+ cliResult.displayText() + " completed in " +
cliResult.elapsedMs() + " ms\n\n" + cliResult.output()));
+ } else {
+ conversation.add(new ConversationEntry(
+ AiRole.ERROR,
+ cliResult.displayText() + " exit code " +
cliResult.exitCode() + "\n\n" + cliResult.output()));
+ }
+ scrollOffset = 0;
+ }
+
+ private void interruptBusyOperation() {
+ if (activeCliCommand != null) {
+ activeCliCommand = null;
+ slashCommandContext.cancelCli();
+ conversation.add(new ConversationEntry(AiRole.SYSTEM, "(command
cancelled)"));
+ thinking.set(false);
+ thinkingVerb = null;
+ } else {
+ stopAgentThread();
+ conversation.add(new ConversationEntry(AiRole.SYSTEM,
"(cancelled)"));
+ }
+ }
+
+ private void stopAgentThread() {
+ Thread t = agentThread;
+ if (t == null) {
+ return;
+ }
+ if (t != Thread.currentThread()) {
+ t.interrupt();
+ // Unlike AiCliCommandExecutor.cancel(), this join is not
offloaded to a watcher thread: it runs
+ // on the caller, which is either the interactive UI thread
(Esc/Ctrl+C via interruptBusyOperation)
+ // or destroy() during shutdown. If the LLM HTTP call in
runAgentLoop() ignores the interrupt (e.g.
+ // a client/library that doesn't check the interrupt flag
mid-request), this blocks that caller for
+ // up to 30s. Left blocking deliberately for now because
AiPanelTest asserts synchronous
+ // cancellation semantics (isAgentThreadRunningForTesting() is
false immediately after Esc); making
+ // this async would need those tests reworked to poll instead.
+ try {
+ t.join(30_000);
Review Comment:
This `t.join(30_000)` runs on the caller's thread (typically the TUI
event/render thread). If an LLM HTTP client ignores the interrupt flag
mid-request, this freezes the entire UI for up to 30 seconds. The code comment
explains the test-driven rationale well, but it's still a user-facing risk.
`AiCliCommandExecutor.cancel()` already solves this same problem by
offloading to a watcher thread — consider applying the same pattern here and
reworking the test assertions to poll with Awaitility (which the project
already uses).
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanel.java:
##########
@@ -323,14 +443,200 @@ boolean handleKeyEvent(KeyEvent ke) {
return true;
}
- private void submitQuestion() {
- String question = inputBuffer.toString().trim();
+ /**
+ * Completes the slash command name at the cursor. With multiple matches,
TAB first fills in the longest common
+ * prefix; once no further prefix can be added it cycles forward through
the matches (wrapping so every match is
+ * reachable with TAB alone). A single match is completed fully and a
trailing space is appended. Shift+TAB cycles
+ * backward. TAB is a no-op unless the buffer is a partial command name
(starts with {@code /}, no arguments yet).
+ */
+ private void handleTabCompletion(boolean backward) {
+ String text = inputBuffer.toString();
+ boolean continuing = text.equals(completionSnapshot) &&
completionMatches != null && completionMatches.size() > 1;
+ if (continuing) {
+ int size = completionMatches.size();
+ if (completionCycleIndex < 0) {
+ completionCycleIndex = backward ? size - 1 : 0;
+ } else {
+ completionCycleIndex = backward
+ ? (completionCycleIndex - 1 + size) % size
+ : (completionCycleIndex + 1) % size;
+ }
+ applyCompletionToken(completionMatches.get(completionCycleIndex),
false);
+ return;
+ }
+
+ List<String> names = slashCommands.completionsFor(text).stream()
+ .map(AiSlashCommandRegistry.Descriptor::name)
+ .toList();
+ if (names.isEmpty()) {
+ completionMatches = null;
+ completionCycleIndex = -1;
+ completionSnapshot = null;
+ return;
+ }
+ if (names.size() == 1) {
+ applyCompletionToken(names.get(0), true);
+ // A single completion ends with a trailing space, so there is
nothing left to cycle.
+ completionMatches = null;
+ completionCycleIndex = -1;
+ completionSnapshot = null;
+ return;
+ }
+ String currentToken = text.substring(1);
+ String prefix = longestCommonPrefix(names);
+ completionMatches = names;
+ if (prefix.length() > currentToken.length()) {
+ applyCompletionToken(prefix, false);
+ completionCycleIndex = -1;
+ } else {
+ completionCycleIndex = backward ? names.size() - 1 : 0;
+ applyCompletionToken(names.get(completionCycleIndex), false);
+ }
+ }
+
+ private void applyCompletionToken(String token, boolean trailingSpace) {
+ inputBuffer.setLength(0);
+ inputBuffer.append('/').append(token);
+ if (trailingSpace) {
+ inputBuffer.append(' ');
+ }
+ cursorPos = inputBuffer.length();
+ completionSnapshot = inputBuffer.toString();
+ }
+
+ private static String longestCommonPrefix(List<String> values) {
+ String prefix = values.get(0);
+ for (int i = 1; i < values.size() && !prefix.isEmpty(); i++) {
+ String value = values.get(i);
+ int max = Math.min(prefix.length(), value.length());
+ int j = 0;
+ while (j < max && prefix.charAt(j) == value.charAt(j)) {
+ j++;
+ }
+ prefix = prefix.substring(0, j);
+ }
+ return prefix;
+ }
+
+ private void submitInput() {
+ String input = inputBuffer.toString().trim();
inputBuffer.setLength(0);
cursorPos = 0;
scrollOffset = 0;
+ if (input.startsWith("/")) {
+ executeSlashCommand(input);
+ } else {
+ submitQuestion(input);
+ }
+ }
+
+ private void executeSlashCommand(String input) {
+ Optional<AiSlashCommandRegistry.ParsedCommand> parsed =
slashCommands.parse(input);
+ if (parsed.isPresent() && thinking.get()) {
+ String name = parsed.get().descriptor().name();
+ if ("provider".equals(name) || "model".equals(name)) {
+ conversation.add(new ConversationEntry(
+ AiRole.SYSTEM,
+ "Wait for the current operation to finish before
changing provider or model."));
+ return;
+ }
+ }
+
+ AiSlashCommandRegistry.CommandResult result =
slashCommands.execute(input, slashCommandContext);
+ if (result.cliRequest() != null) {
+ AiCliCommandExecutor.Request request = result.cliRequest();
+ conversation.add(new ConversationEntry(AiRole.SYSTEM, "Running " +
request.displayText()));
+ // Runs in the background without entering the panel's "thinking"
state, so the user can keep typing
+ // and asking questions while the command runs. Esc still cancels
it via interruptBusyOperation().
+ CompletableFuture<AiCliCommandExecutor.Result> future =
slashCommandContext.executeCli(request);
+ activeCliCommand = future;
+ future.whenComplete(this::handleCliCompletion);
+ return;
+ }
+ if (result.text() != null && !result.text().isBlank()) {
+ conversation.add(new ConversationEntry(result.role(),
result.text()));
+ }
+ }
+
+ private void handleCliCompletion(AiCliCommandExecutor.Result cliResult,
Throwable error) {
+ CompletableFuture<AiCliCommandExecutor.Result> future =
activeCliCommand;
+ synchronized (this) {
+ if (future == null || activeCliCommand != future) {
+ return;
+ }
+ activeCliCommand = null;
+ }
+ thinking.set(false);
+ thinkingVerb = null;
Review Comment:
This unconditionally clears `thinking` and `thinkingVerb` when a CLI command
completes. But CLI commands (like `/send`) are designed to run _without_
putting the panel into the thinking state, so the user can keep typing and even
submit an LLM question while a CLI command is in-flight.
If a user does `/send direct:foo hello` and then immediately asks a question
(which sets `thinking=true`), this callback could race and clear the thinking
state while the LLM agent thread is still running — leaving the panel appearing
idle with no spinner.
Consider guarding this with `if (activeCliCommand != null)` or only clearing
thinking if it was set by the CLI path.
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/LlmClient.java:
##########
@@ -1126,11 +1214,41 @@ private String sendAnthropicStreamingRequest(String
requestUrl, JsonObject body,
// ---- Endpoint detection helpers ----
+ // Order matters: /api/tags is checked before /v1/models, matching the
original priority.
+ private static final List<Map.Entry<String, ApiType>>
EXPLICIT_URL_HEALTH_CHECK_SUFFIXES = List.of(
+ Map.entry("/api/tags", ApiType.ollama),
+ Map.entry("/v1/models", ApiType.openai));
+
private boolean tryExplicitUrl() {
if (url == null || url.isBlank()) {
return false;
}
- return isEndpointReachable(url);
+ for (Map.Entry<String, ApiType> check :
EXPLICIT_URL_HEALTH_CHECK_SUFFIXES) {
+ if (tryHealthCheck(url + check.getKey())) {
+ if (apiType == null) {
+ apiType = check.getValue();
+ }
+ return true;
+ }
+ }
+ if (tryHealthCheck(url)) {
+ if (apiType == null) {
+ apiType = inferApiTypeFromUrlHost();
+ }
+ return true;
+ }
+ return false;
+ }
+
+ ApiType inferApiTypeFromUrlHost() {
+ String lower = url.toLowerCase();
+ if (lower.contains("anthropic")) {
+ return ApiType.anthropic;
+ }
+ if (lower.contains("11434") || lower.contains("ollama")) {
+ return ApiType.ollama;
+ }
+ return ApiType.openai;
Review Comment:
The original `tryExplicitUrl()` was a pure query (just checked reachability
without mutating state). This new version now silently sets `apiType` as a side
effect when it was previously null, using URL-based heuristics that default to
`openai` for anything that doesn't match "anthropic", "11434", or "ollama".
A few things worth considering:
- Custom proxy endpoints (e.g. `https://gateway.internal/llm`) would be
classified as `openai` even if they proxy Ollama or Anthropic.
- The other callers of `isEndpointReachable()` (used by `tryOpenAi()`,
`tryOllama()`, etc.) do _not_ infer `apiType`, so the detection behavior is now
inconsistent depending on the entry point.
The PR body says this was verified and the heuristic only fires when
`apiType` was unset. Just flagging for awareness — if this causes
misclassification reports, this is the place to look.
--
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]
