davsclaus commented on code in PR #24594:
URL: https://github.com/apache/camel/pull/24594#discussion_r3563387846
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/AiPanel.java:
##########
@@ -83,21 +94,37 @@ record LogEntry(String timestamp, LogLevel level, String
message, String detail)
private final StringBuilder inputBuffer = new StringBuilder();
private int cursorPos;
- // Conversation display
+ // Conversation display. CopyOnWriteArrayList because entries are appended
from the agent thread and the
+ // CLI-command-completion callback while the render thread iterates the
list concurrently.
private final List<ConversationEntry> conversation = new
CopyOnWriteArrayList<>();
private int scrollOffset;
// LLM state
- private LlmClient client;
+ private volatile LlmClient client;
private List<LlmClient.Message> messages;
private List<LlmClient.ToolDef> tools;
private AskTools askTools;
private final AtomicBoolean thinking = new AtomicBoolean();
private volatile Thread agentThread;
private String initError;
private long thinkingStartTime;
+ private String thinkingVerb;
private volatile int sessionTotalTokens;
Review Comment:
**Data race on `thinkingVerb`:** This field is a plain `private String`
written from the UI thread (in `submitQuestion` / `executeSlashCommand`) and
read/written from the `CompletableFuture` completion callback thread (in
`handleCliCompletion`). Without `volatile`, the JMM does not guarantee
visibility across threads.
Practically harmless (display-only), but easy to fix:
```suggestion
private volatile String thinkingVerb;
```
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/LlmClient.java:
##########
@@ -287,6 +287,95 @@ public ChatResponse chatWithTools(String systemPrompt,
List<Message> messages, L
};
}
+ // -- Model listing --
+
+ /**
+ * Lists model identifiers available from the configured provider:
Ollama's {@code /api/tags}, or the
+ * OpenAI-compatible/Anthropic {@code /v1/models}. Vertex AI has no
equivalent listing endpoint (models come from
+ * the Vertex AI model garden, not the Anthropic API surface), so it
always returns an empty list. Best-effort: any
+ * failure (unreachable endpoint, non-200 response, unexpected JSON shape)
yields an empty list rather than
+ * throwing, since this only feeds the informational {@code /model}
listing in the AI panel.
+ */
+ public List<String> listModels() {
+ if (apiType == null || url == null || url.isBlank()) {
Review Comment:
**Behavioral change in endpoint detection:** The old `tryExplicitUrl()`
simply called `isEndpointReachable(url)`. The new version probes `url +
"/api/tags"` and `url + "/v1/models"` first, and silently sets `apiType` as a
side effect when a probe succeeds.
This could change behavior for users with custom proxy/gateway URLs that
don't serve those sub-paths (the probes would fail, falling through to the bare
URL check, but the extra network round-trips add latency). Worth noting in the
JIRA or commit message that detection behavior changed.
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/Ask.java:
##########
@@ -122,7 +122,8 @@ public Integer doCall() throws Exception {
if (!client.detectEndpoint()) {
printer().printErr("LLM service is not reachable.");
- printer().printErr("Options: --url=<endpoint>,
--api-type=anthropic, or start Ollama with: camel infra run ollama");
+ printer()
Review Comment:
Nit: This is an unrelated formatter change (line wrap of `printErr`) mixed
into the feature PR. Not wrong, but ideally would be a separate commit or
omitted to keep the diff focused on the slash-commands feature.
--
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]
