From 5e6636d3b9d038130255db968e835a75e4b5f2ab Mon Sep 17 00:00:00 2001 From: Lucio Lelii Date: Thu, 17 Sep 2026 12:28:46 +0200 Subject: [PATCH] Extract Ollama's protocol into a shared base; add a remote Ollama provider InternalOllamaLLMProvider carried its own client, its own error mapping and logging, and its own request bodies and parsing, all mixed together. OllamaProtocolProvider pulls out everything that is genuinely the protocol - nested options, think:true, top_k, the JSON-path defaults the flow assistant depends on, response parsing - onto the transport base, leaving InternalOllamaLLMProvider about fifty lines: its own URL, its own key, and nothing else. getName() still returns "InternalOllama" - that string is persisted in seventeen places in workflow-editor-init/flows.json, in every existing flow, and in the vault's provider column, so it could not change even in a refactor this size. Not extended from OpenAIProtocolProvider, even though Ollama also exposes an OpenAI-compatible endpoint: the native shape differs enough - nested options, think, top_k, none of which OpenAI has - that a subclass would override every method the parent provides, which is not a subclass, it is a different implementation wearing one. The base ended up with two hooks instead of the OpenAI family's one, because there is a real asymmetry here that family does not have: resolveApiKey() lets InternalOllamaLLMProvider ignore whatever credential a caller passes and always use its own server-configured key, while RemoteOllamaProvider - the new provider, for an Ollama instance other than our own - requires the caller's. baseUrl() has the same shape as OpenAICompatibleProvider's: a constant for the internal instance, read from the credential (and validated through OutboundEndpointGuard) for the remote one. RemoteOllamaProvider cannot list its models either, for the same reason OpenAICompatibleProvider cannot: listing would run from the editor, with no credential and therefore no endpoint to ask. InternalOllamaLLMProviderBodyTest - the existing test pinning every request body byte for byte - passes unchanged, which is what "extraction" is supposed to mean here. InternalOllamaLLMProviderHttpTest is new: no test before this one exercised the actual HTTP round trip, only the bodies, so there was no way to confirm the 4xx-carries-its-body upgrade (the whole reason for building the shared base) actually reached Ollama until now. Co-Authored-By: Claude Sonnet 5 --- .../ollama/InternalOllamaLLMProvider.java | 354 ++---------------- .../ollama/OllamaProtocolProvider.java | 279 ++++++++++++++ .../ollama/RemoteOllamaProvider.java | 83 ++++ .../InternalOllamaLLMProviderHttpTest.java | 123 ++++++ .../ollama/RemoteOllamaProviderTest.java | 79 ++++ 5 files changed, 599 insertions(+), 319 deletions(-) create mode 100644 src/main/java/it/cnr/isti/workflow/manager/llms/providers/ollama/OllamaProtocolProvider.java create mode 100644 src/main/java/it/cnr/isti/workflow/manager/llms/providers/ollama/RemoteOllamaProvider.java create mode 100644 src/test/java/it/cnr/isti/workflow/manager/llms/providers/ollama/InternalOllamaLLMProviderHttpTest.java create mode 100644 src/test/java/it/cnr/isti/workflow/manager/llms/providers/ollama/RemoteOllamaProviderTest.java diff --git a/src/main/java/it/cnr/isti/workflow/manager/llms/providers/ollama/InternalOllamaLLMProvider.java b/src/main/java/it/cnr/isti/workflow/manager/llms/providers/ollama/InternalOllamaLLMProvider.java index 481a540..31a3ada 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/llms/providers/ollama/InternalOllamaLLMProvider.java +++ b/src/main/java/it/cnr/isti/workflow/manager/llms/providers/ollama/InternalOllamaLLMProvider.java @@ -4,340 +4,56 @@ package it.cnr.isti.workflow.manager.llms.providers.ollama; -import java.time.Duration; -import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; import java.util.Objects; -import java.util.concurrent.TimeoutException; -import org.slf4j.Logger; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.beans.factory.annotation.Value; -import org.springframework.http.MediaType; import org.springframework.stereotype.Service; import org.springframework.web.reactive.function.client.WebClient; -import org.springframework.web.reactive.function.client.WebClientRequestException; -import org.springframework.web.reactive.function.client.WebClientResponseException; -import tools.jackson.databind.ObjectMapper; -import it.cnr.isti.workflow.manager.commons.Throwables; -import it.cnr.isti.workflow.manager.llms.ChatMessage; -import it.cnr.isti.workflow.manager.llms.ModelParameters; -import it.cnr.isti.workflow.manager.llms.providers.LLMProvider; -import it.cnr.isti.workflow.manager.llms.providers.ollama.response.ChatResponse; -import it.cnr.isti.workflow.manager.llms.providers.ollama.response.GenerateResponse; -import it.cnr.isti.workflow.manager.llms.providers.ollama.response.ModelInfo; -import it.cnr.isti.workflow.manager.llms.providers.ollama.response.ModelResponse; -import reactor.core.publisher.Mono; +import it.cnr.isti.workflow.manager.llms.ProviderCredential; +/** + * The institute's own Ollama instance: base URL and API key are both server configuration, not + * anything a user supplies - so it needs no credential ({@link #requiresAuthorization()} stays the + * interface's own {@code false}) and no endpoint ({@link #requiresEndpoint()} likewise). Its name is + * load-bearing and must never change: it is persisted in every existing flow, in + * {@code workflow-editor-init/flows.json}, and in the vault's {@code provider} column. + */ @Service -public class InternalOllamaLLMProvider implements LLMProvider { +public class InternalOllamaLLMProvider extends OllamaProtocolProvider { - private static final Logger log = org.slf4j.LoggerFactory.getLogger(InternalOllamaLLMProvider.class); - private static final int MAX_ERROR_BODY_LOG_LENGTH = 1_000; - private static final ObjectMapper objectMapper = new ObjectMapper(); - private String ollamaKey; + private final String ollamaKey; + private final String ollamaURL; - private final WebClient.Builder webClientBuilder; - private final WebClient webClient; - private String ollamaURL; + public InternalOllamaLLMProvider(@Qualifier("llmWebClientBuilder") WebClient.Builder webClientBuilder, + @Value("${app.ollama.internal.key}") String ollamaKey, + @Value("${app.ollama.internal.url}") String ollamaUri) { + super(webClientBuilder); + this.ollamaKey = Objects.requireNonNull(ollamaKey, "ollamaKey cannot be null"); + this.ollamaURL = Objects.requireNonNull(ollamaUri, "ollamaUri cannot be null"); + } - public InternalOllamaLLMProvider(@Qualifier("llmWebClientBuilder") WebClient.Builder webClientBuilder, - @Value("${app.ollama.internal.key}") String ollamakey, - @Value("${app.ollama.internal.url}") String ollamaUri) { - Objects.requireNonNull(ollamakey, "ollamaKey cannot be null"); - Objects.requireNonNull(ollamaUri, "ollamaUri cannot be null"); - this.ollamaKey = ollamakey; - this.ollamaURL = ollamaUri; + @Override + public String getName() { + return "InternalOllama"; + } - this.webClientBuilder = webClientBuilder; - this.webClient = webClientBuilder.baseUrl(this.ollamaURL).build(); - } + @Override + protected String baseUrl(ProviderCredential credential) { + return ollamaURL; + } - @Override - public String getName() { - return "InternalOllama"; - } + /** Ignores whatever is passed: this key is the server's own, never a per-user credential. */ + @Override + protected String resolveApiKey(ProviderCredential credential) { + return ollamaKey; + } - public String generate(String model, String prompt) { - return generate(model, prompt, false, null); - } - - @Override - public String generateJson(String model, String prompt) { - return generate(model, prompt, true, null); - } - - @Override - public String generate(String model, String prompt, String authorization, ModelParameters parameters) { - return generate(model, prompt, false, parameters); - } - - @Override - public String generateJson(String model, String prompt, String authorization, ModelParameters parameters) { - return generate(model, prompt, true, parameters); - } - - /** - * Ollama's own names for the sampling options. Kept in one place so the chat path and the - * generate path cannot drift. - */ - private static Map optionsFor(ModelParameters parameters) { - Map options = new LinkedHashMap<>(); - if (ModelParameters.isEmpty(parameters)) { - return options; - } - if (parameters.temperature() != null) options.put("temperature", parameters.temperature()); - if (parameters.topP() != null) options.put("top_p", parameters.topP()); - if (parameters.topK() != null) options.put("top_k", parameters.topK()); - if (parameters.maxTokens() != null) options.put("num_predict", parameters.maxTokens()); - if (parameters.seed() != null) options.put("seed", parameters.seed()); - return options; - } - - /** - * Package-private so the request body can be asserted without a server, the way - * MCPAgentService.buildOpenSessionRequest is. The defaults it carries are load-bearing. - */ - Map buildGenerateBody(String model, String prompt, boolean jsonResponse, - ModelParameters parameters) { - Map bodyMap = new LinkedHashMap<>(); - bodyMap.put("model", model); - bodyMap.put("prompt", prompt); - bodyMap.put("stream", false); - // A thinking model (qwen3, deepseek-r1, ...) that isn't asked to think explicitly still - // reasons under the hood on some prompts and mixes the reasoning trace into "response" - // itself. Asking for it explicitly makes Ollama return it separately as "thinking" - // instead, which parseGenerateResponse below never reads - so "response" stays just the - // final answer, with no loss of reasoning quality (unlike think:false, which would ask - // the model to skip reasoning altogether). Ignored by models/versions that don't support it. - bodyMap.put("think", true); - if (jsonResponse) { - bodyMap.put("format", "json"); - // These two have been forced on the JSON path since before parameters existed, and - // the flow assistant's output depends on them. They stay as the baseline; a value - // the caller sets overrides its own field and nothing else. - Map options = new LinkedHashMap<>(); - options.put("temperature", 0.1); - options.put("num_predict", 4096); - options.putAll(optionsFor(parameters)); - bodyMap.put("options", options); - } else { - // The text path has never sent options, so with nothing set it still sends none. - Map options = optionsFor(parameters); - if (!options.isEmpty()) { - bodyMap.put("options", options); - } - } - return bodyMap; - } - - /** Mutable, unlike the Map.of it replaces: with nothing set the body is the one it always sent. */ - Map buildChatBody(String model, List messages, ModelParameters parameters) { - Map bodyMap = new LinkedHashMap<>(); - bodyMap.put("model", model); - bodyMap.put("messages", messages.stream() - .map(message -> Map.of( - "role", message.role().name().toLowerCase(), - "content", message.content())) - .toList()); - bodyMap.put("stream", false); - // See buildGenerateBody: keeps reasoning out of message.content without asking the model - // to reason less. - bodyMap.put("think", true); - Map options = optionsFor(parameters); - if (!options.isEmpty()) { - bodyMap.put("options", options); - } - return bodyMap; - } - - private String generate(String model, String prompt, boolean jsonResponse, ModelParameters parameters) { - Objects.requireNonNull(prompt, "prompt cannot be null"); - Objects.requireNonNull(model, "model cannot be null"); - Map bodyMap = buildGenerateBody(model, prompt, jsonResponse, parameters); - - Mono result = webClient.post() - .uri(uriBuilder -> uriBuilder.pathSegment("generate") - .build()) - .header("Authorization", "Bearer " + ollamaKey) - .contentType(MediaType.APPLICATION_JSON) - .bodyValue(bodyMap) - .retrieve() - .onStatus( - status -> status.is5xxServerError(), - clientResponse -> clientResponse.bodyToMono(String.class) - .defaultIfEmpty("Error: server without body") - .flatMap(body -> Mono.error(new RuntimeException( - "Error 5xx: " + body)))) - .bodyToMono(String.class) - .timeout(Duration.ofMinutes(2)); - - return parseGenerateResponse(blockOllamaCall(result, "generate", model, jsonResponse), objectMapper); - } - - @Override - public String chat(String model, List messages) { - return chat(model, messages, null, null); - } - - @Override - public String chat(String model, List messages, String authorization, - ModelParameters parameters) { - Objects.requireNonNull(messages, "messages cannot be null"); - Objects.requireNonNull(model, "model cannot be null"); - Map bodyMap = buildChatBody(model, messages, parameters); - - Mono result = webClient.post() - .uri(uriBuilder -> uriBuilder.pathSegment("chat").build()) - .header("Authorization", "Bearer " + ollamaKey) - .contentType(MediaType.APPLICATION_JSON) - .bodyValue(bodyMap) - .retrieve() - .onStatus( - status -> status.is5xxServerError(), - clientResponse -> clientResponse.bodyToMono(String.class) - .defaultIfEmpty("Error: server without body") - .flatMap(body -> Mono.error(new RuntimeException( - "Error 5xx: " + body)))) - .bodyToMono(String.class) - .timeout(Duration.ofMinutes(2)); - - return parseChatResponse(blockOllamaCall(result, "chat", model, false), objectMapper); - } - - private String parseGenerateResponse(String responseBody, ObjectMapper mapper) { - if (responseBody == null || responseBody.isBlank()) { - throw new RuntimeException("Empty response body from Ollama generate endpoint"); - } - - String trimmedResponse = responseBody.trim(); - if (trimmedResponse.startsWith("{")) { - try { - GenerateResponse response = mapper.readValue(trimmedResponse, GenerateResponse.class); - if (response.getResponse() == null) { - throw new RuntimeException("Missing 'response' field in Ollama JSON response"); - } - return response.getResponse(); - } catch (Exception e) { - throw new RuntimeException("Unable to parse Ollama JSON response", e); - } - } - - log.debug("Ollama generate endpoint returned text/plain response"); - return trimmedResponse; - } - - private String parseChatResponse(String responseBody, ObjectMapper mapper) { - if (responseBody == null || responseBody.isBlank()) { - throw new RuntimeException("Empty response body from Ollama chat endpoint"); - } - - String trimmedResponse = responseBody.trim(); - if (trimmedResponse.startsWith("{")) { - try { - ChatResponse response = mapper.readValue(trimmedResponse, ChatResponse.class); - if (response.getMessage() == null || response.getMessage().getContent() == null) { - throw new RuntimeException("Missing 'message.content' field in Ollama chat response"); - } - return response.getMessage().getContent(); - } catch (Exception e) { - throw new RuntimeException("Unable to parse Ollama chat response", e); - } - } - - log.debug("Ollama chat endpoint returned text/plain response"); - return trimmedResponse; - } - - public List getRegisteredModels() { - Mono> result = webClient.get() - .uri(uriBuilder -> uriBuilder.pathSegment("tags") - .build()) - .header("Authorization", "Bearer " + ollamaKey) - .retrieve() - .onStatus( - status -> status.is5xxServerError(), - clientResponse -> clientResponse.bodyToMono(String.class) - .defaultIfEmpty("Error: server without body") - .flatMap(body -> Mono.error(new RuntimeException( - "Error 5xx: " + body)))) - .bodyToMono(ModelResponse.class) // deserialize JSON in oggetto Java - .timeout(Duration.ofMinutes(1)) - .map(modelResponse -> modelResponse.getModels().stream() - .map(ModelInfo::getName) - .toList()); - - return blockOllamaCall(result, "tags", null, false); - } - - private T blockOllamaCall(Mono result, String endpoint, String model, boolean jsonResponse) { - try { - return result.block(); - } catch (RuntimeException e) { - logOllamaFailure(endpoint, model, jsonResponse, e); - throw e; - } - } - - private void logOllamaFailure(String endpoint, String model, boolean jsonResponse, RuntimeException error) { - Throwable root = Throwables.rootCause(error); - String modelLabel = model == null ? "(none)" : model; - if (error instanceof WebClientResponseException responseError) { - log.error( - "Ollama provider HTTP error. endpoint={}, model={}, jsonResponse={}, status={}, body={}", - endpoint, - modelLabel, - jsonResponse, - responseError.getStatusCode(), - abbreviate(responseError.getResponseBodyAsString()), - error); - return; - } - if (error instanceof WebClientRequestException) { - log.error( - "Ollama provider connection error. endpoint={}, model={}, jsonResponse={}, ollamaUrl={}, errorType={}, message={}", - endpoint, - modelLabel, - jsonResponse, - ollamaURL, - root.getClass().getSimpleName(), - root.getMessage(), - error); - return; - } - if (root instanceof TimeoutException) { - log.error( - "Ollama provider timeout. endpoint={}, model={}, jsonResponse={}, ollamaUrl={}, message={}", - endpoint, - modelLabel, - jsonResponse, - ollamaURL, - root.getMessage(), - error); - return; - } - log.error( - "Ollama provider call failed. endpoint={}, model={}, jsonResponse={}, ollamaUrl={}, errorType={}, message={}", - endpoint, - modelLabel, - jsonResponse, - ollamaURL, - root.getClass().getSimpleName(), - root.getMessage(), - error); - } - - private String abbreviate(String value) { - if (value == null || value.isBlank()) { - return "(empty)"; - } - String normalized = value.replaceAll("\\s+", " ").trim(); - if (normalized.length() <= MAX_ERROR_BODY_LOG_LENGTH) { - return normalized; - } - return normalized.substring(0, MAX_ERROR_BODY_LOG_LENGTH) + "..."; - } + @Override + public List getRegisteredModels() { + return fetchRegisteredModels(null); + } } diff --git a/src/main/java/it/cnr/isti/workflow/manager/llms/providers/ollama/OllamaProtocolProvider.java b/src/main/java/it/cnr/isti/workflow/manager/llms/providers/ollama/OllamaProtocolProvider.java new file mode 100644 index 0000000..e525e37 --- /dev/null +++ b/src/main/java/it/cnr/isti/workflow/manager/llms/providers/ollama/OllamaProtocolProvider.java @@ -0,0 +1,279 @@ +// SPDX-FileCopyrightText: 2025-2026 Lucio Lelii - ISTI-CNR +// SPDX-License-Identifier: AGPL-3.0-or-later +// Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM. + +package it.cnr.isti.workflow.manager.llms.providers.ollama; + +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.http.MediaType; +import org.springframework.web.reactive.function.client.WebClient; + +import tools.jackson.databind.ObjectMapper; +import it.cnr.isti.workflow.manager.llms.ChatMessage; +import it.cnr.isti.workflow.manager.llms.ModelParameters; +import it.cnr.isti.workflow.manager.llms.ProviderCredential; +import it.cnr.isti.workflow.manager.llms.providers.AbstractHttpLLMProvider; +import it.cnr.isti.workflow.manager.llms.providers.ollama.response.ChatResponse; +import it.cnr.isti.workflow.manager.llms.providers.ollama.response.GenerateResponse; +import it.cnr.isti.workflow.manager.llms.providers.ollama.response.ModelInfo; +import it.cnr.isti.workflow.manager.llms.providers.ollama.response.ModelResponse; +import reactor.core.publisher.Mono; + +/** + * Ollama's own request/response shape - nested {@code options}, {@code think}, {@code top_k} - shared + * by every provider that speaks it: our own internal instance and a remote one a user points at. + * Deliberately not a subtype of {@code OpenAIProtocolProvider}, even though Ollama also exposes an + * OpenAI-compatible endpoint: extending it would mean overriding path, body, parsing and supported + * parameters wholesale, and losing {@code think} and {@code top_k} in the process - see + * {@code docs/llm-providers-openai-remote-ollama-plan-2026-09-17.md}. + * + *

Only the base URL and which key authenticates differ between the two concrete providers: our + * own instance has both baked in from server configuration, and a remote one reads both from the + * resolved credential. Everything else - the two hooks below aside - is identical, including the + * defaults this interface already provides: {@code canListModels()} true and every + * {@code ModelParameter} supported, both of which our own instance keeps and a remote one narrows. + */ +public abstract class OllamaProtocolProvider extends AbstractHttpLLMProvider { + + private static final Logger log = LoggerFactory.getLogger(OllamaProtocolProvider.class); + private final ObjectMapper objectMapper = new ObjectMapper(); + + protected OllamaProtocolProvider(WebClient.Builder webClientBuilder) { + super(webClientBuilder); + } + + /** The base URL to call - a server-configured constant, or read from the credential. */ + protected abstract String baseUrl(ProviderCredential credential); + + /** + * The key that goes in the {@code Authorization: Bearer} header. Our own instance ignores + * whatever is passed and returns its own server-configured key - it is not tied to any user's + * credential - while a remote instance requires the credential's own value. + */ + protected abstract String resolveApiKey(ProviderCredential credential); + + @Override + public String generate(String model, String prompt) { + return generateCall(model, prompt, false, null, null); + } + + @Override + public String generateJson(String model, String prompt) { + return generateCall(model, prompt, true, null, null); + } + + @Override + public String generate(String model, String prompt, String authorization) { + return generateCall(model, prompt, false, ProviderCredential.ofValue(authorization), null); + } + + @Override + public String generateJson(String model, String prompt, String authorization) { + return generateCall(model, prompt, true, ProviderCredential.ofValue(authorization), null); + } + + @Override + public String generate(String model, String prompt, String authorization, ModelParameters parameters) { + return generateCall(model, prompt, false, ProviderCredential.ofValue(authorization), parameters); + } + + @Override + public String generateJson(String model, String prompt, String authorization, ModelParameters parameters) { + return generateCall(model, prompt, true, ProviderCredential.ofValue(authorization), parameters); + } + + @Override + public String generate(String model, String prompt, ProviderCredential credential, ModelParameters parameters) { + return generateCall(model, prompt, false, credential, parameters); + } + + @Override + public String generateJson(String model, String prompt, ProviderCredential credential, ModelParameters parameters) { + return generateCall(model, prompt, true, credential, parameters); + } + + @Override + public String chat(String model, List messages) { + return chatCall(model, messages, null, null); + } + + @Override + public String chat(String model, List messages, String authorization) { + return chatCall(model, messages, ProviderCredential.ofValue(authorization), null); + } + + @Override + public String chat(String model, List messages, String authorization, ModelParameters parameters) { + return chatCall(model, messages, ProviderCredential.ofValue(authorization), parameters); + } + + @Override + public String chat(String model, List messages, ProviderCredential credential, + ModelParameters parameters) { + return chatCall(model, messages, credential, parameters); + } + + private String generateCall(String model, String prompt, boolean jsonResponse, ProviderCredential credential, + ModelParameters parameters) { + Objects.requireNonNull(model, "model cannot be null"); + Objects.requireNonNull(prompt, "prompt cannot be null"); + Map body = buildGenerateBody(model, prompt, jsonResponse, parameters); + Mono request = withErrorHandling(clientFor(baseUrl(credential)).post() + .uri("/generate") + .header("Authorization", "Bearer " + resolveApiKey(credential)) + .contentType(MediaType.APPLICATION_JSON) + .bodyValue(body) + .retrieve()); + return parseGenerateResponse(blockingCall(request, "generate", model)); + } + + private String chatCall(String model, List messages, ProviderCredential credential, + ModelParameters parameters) { + Objects.requireNonNull(model, "model cannot be null"); + Objects.requireNonNull(messages, "messages cannot be null"); + Map body = buildChatBody(model, messages, parameters); + Mono request = withErrorHandling(clientFor(baseUrl(credential)).post() + .uri("/chat") + .header("Authorization", "Bearer " + resolveApiKey(credential)) + .contentType(MediaType.APPLICATION_JSON) + .bodyValue(body) + .retrieve()); + return parseChatResponse(blockingCall(request, "chat", model)); + } + + /** + * Only for a subtype whose {@code canListModels()} is true - our own instance calls this from + * {@code getRegisteredModels()}; a remote instance, which cannot list without a credential the + * editor does not have, never calls it and returns an empty list instead. + */ + protected final List fetchRegisteredModels(ProviderCredential credential) { + Mono request = withErrorHandling(clientFor(baseUrl(credential)).get() + .uri("/tags") + .header("Authorization", "Bearer " + resolveApiKey(credential)) + .retrieve()); + String body = blockingCall(request, "tags", null); + try { + ModelResponse response = objectMapper.readValue(body, ModelResponse.class); + return response.getModels().stream().map(ModelInfo::getName).toList(); + } catch (RuntimeException e) { + throw new RuntimeException("Unable to parse Ollama tags response: " + abbreviate(body), e); + } + } + + /** + * Ollama's own names for the sampling options. Kept in one place so the chat path and the + * generate path cannot drift. + */ + private static Map optionsFor(ModelParameters parameters) { + Map options = new LinkedHashMap<>(); + if (ModelParameters.isEmpty(parameters)) { + return options; + } + if (parameters.temperature() != null) options.put("temperature", parameters.temperature()); + if (parameters.topP() != null) options.put("top_p", parameters.topP()); + if (parameters.topK() != null) options.put("top_k", parameters.topK()); + if (parameters.maxTokens() != null) options.put("num_predict", parameters.maxTokens()); + if (parameters.seed() != null) options.put("seed", parameters.seed()); + return options; + } + + /** Package-private so the request body can be asserted without a server. The defaults it carries are load-bearing. */ + Map buildGenerateBody(String model, String prompt, boolean jsonResponse, ModelParameters parameters) { + Map bodyMap = new LinkedHashMap<>(); + bodyMap.put("model", model); + bodyMap.put("prompt", prompt); + bodyMap.put("stream", false); + // A thinking model (qwen3, deepseek-r1, ...) that isn't asked to think explicitly still + // reasons under the hood on some prompts and mixes the reasoning trace into "response" + // itself. Asking for it explicitly makes Ollama return it separately as "thinking" instead, + // which parseGenerateResponse below never reads - so "response" stays just the final + // answer, with no loss of reasoning quality (unlike think:false, which would ask the model + // to skip reasoning altogether). Ignored by models/versions that don't support it. + bodyMap.put("think", true); + if (jsonResponse) { + bodyMap.put("format", "json"); + // These two have been forced on the JSON path since before parameters existed, and the + // flow assistant's output depends on them. They stay as the baseline; a value the + // caller sets overrides its own field and nothing else. + Map options = new LinkedHashMap<>(); + options.put("temperature", 0.1); + options.put("num_predict", 4096); + options.putAll(optionsFor(parameters)); + bodyMap.put("options", options); + } else { + // The text path has never sent options, so with nothing set it still sends none. + Map options = optionsFor(parameters); + if (!options.isEmpty()) { + bodyMap.put("options", options); + } + } + return bodyMap; + } + + /** Mutable, unlike the Map.of it replaces: with nothing set the body is the one it always sent. */ + Map buildChatBody(String model, List messages, ModelParameters parameters) { + Map bodyMap = new LinkedHashMap<>(); + bodyMap.put("model", model); + bodyMap.put("messages", messages.stream() + .map(message -> Map.of( + "role", message.role().name().toLowerCase(), + "content", message.content())) + .toList()); + bodyMap.put("stream", false); + // See buildGenerateBody: keeps reasoning out of message.content without asking the model to + // reason less. + bodyMap.put("think", true); + Map options = optionsFor(parameters); + if (!options.isEmpty()) { + bodyMap.put("options", options); + } + return bodyMap; + } + + private String parseGenerateResponse(String responseBody) { + if (responseBody == null || responseBody.isBlank()) { + throw new RuntimeException("Empty response body from Ollama generate endpoint"); + } + String trimmedResponse = responseBody.trim(); + if (trimmedResponse.startsWith("{")) { + try { + GenerateResponse response = objectMapper.readValue(trimmedResponse, GenerateResponse.class); + if (response.getResponse() == null) { + throw new RuntimeException("Missing 'response' field in Ollama JSON response"); + } + return response.getResponse(); + } catch (Exception e) { + throw new RuntimeException("Unable to parse Ollama JSON response", e); + } + } + log.debug("Ollama generate endpoint returned text/plain response"); + return trimmedResponse; + } + + private String parseChatResponse(String responseBody) { + if (responseBody == null || responseBody.isBlank()) { + throw new RuntimeException("Empty response body from Ollama chat endpoint"); + } + String trimmedResponse = responseBody.trim(); + if (trimmedResponse.startsWith("{")) { + try { + ChatResponse response = objectMapper.readValue(trimmedResponse, ChatResponse.class); + if (response.getMessage() == null || response.getMessage().getContent() == null) { + throw new RuntimeException("Missing 'message.content' field in Ollama chat response"); + } + return response.getMessage().getContent(); + } catch (Exception e) { + throw new RuntimeException("Unable to parse Ollama chat response", e); + } + } + log.debug("Ollama chat endpoint returned text/plain response"); + return trimmedResponse; + } + +} diff --git a/src/main/java/it/cnr/isti/workflow/manager/llms/providers/ollama/RemoteOllamaProvider.java b/src/main/java/it/cnr/isti/workflow/manager/llms/providers/ollama/RemoteOllamaProvider.java new file mode 100644 index 0000000..dc5abdf --- /dev/null +++ b/src/main/java/it/cnr/isti/workflow/manager/llms/providers/ollama/RemoteOllamaProvider.java @@ -0,0 +1,83 @@ +// SPDX-FileCopyrightText: 2025-2026 Lucio Lelii - ISTI-CNR +// SPDX-License-Identifier: AGPL-3.0-or-later +// Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM. + +package it.cnr.isti.workflow.manager.llms.providers.ollama; + +import java.util.List; + +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.stereotype.Service; +import org.springframework.web.reactive.function.client.WebClient; + +import it.cnr.isti.workflow.manager.llms.ProviderCredential; +import it.cnr.isti.workflow.manager.llms.providers.OutboundEndpointGuard; + +/** + * An Ollama instance other than the institute's own - a colleague's workstation, a lab GPU box - whose + * address and key the user supplies on the credential, the same way {@code OpenAICompatibleProvider} + * does. Not listable: {@code getRegisteredModels()} runs from the editor, with no credential and + * therefore no endpoint to ask, so the model field is free text - the same choice already made for + * every provider that cannot enumerate without one. + */ +@Service +public class RemoteOllamaProvider extends OllamaProtocolProvider { + + private final OutboundEndpointGuard endpointGuard; + + public RemoteOllamaProvider(@Qualifier("llmWebClientBuilder") WebClient.Builder webClientBuilder, + OutboundEndpointGuard endpointGuard) { + super(webClientBuilder); + this.endpointGuard = endpointGuard; + } + + @Override + public String getName() { + return "RemoteOllama"; + } + + @Override + public boolean requiresEndpoint() { + return true; + } + + @Override + public boolean requiresAuthorization() { + return true; + } + + @Override + public String authorizationDescription() { + return "API key for the remote Ollama instance. Any non-blank value works if the server does not check one."; + } + + @Override + public boolean canListModels() { + return false; + } + + @Override + public List getRegisteredModels() { + return List.of(); + } + + /** The only place the "no endpoint" case is rejected - see OpenAICompatibleProvider's own note. */ + @Override + protected String baseUrl(ProviderCredential credential) { + String endpoint = credential == null ? null : credential.endpoint(); + if (endpoint == null || endpoint.isBlank()) { + throw new IllegalArgumentException(getName() + " requires an endpoint from the saved credential"); + } + endpointGuard.validate(endpoint); + return endpoint; + } + + @Override + protected String resolveApiKey(ProviderCredential credential) { + if (credential == null || credential.value() == null) { + throw new IllegalArgumentException(getName() + " requires a credential"); + } + return credential.value(); + } + +} diff --git a/src/test/java/it/cnr/isti/workflow/manager/llms/providers/ollama/InternalOllamaLLMProviderHttpTest.java b/src/test/java/it/cnr/isti/workflow/manager/llms/providers/ollama/InternalOllamaLLMProviderHttpTest.java new file mode 100644 index 0000000..4ec6a48 --- /dev/null +++ b/src/test/java/it/cnr/isti/workflow/manager/llms/providers/ollama/InternalOllamaLLMProviderHttpTest.java @@ -0,0 +1,123 @@ +// SPDX-FileCopyrightText: 2025-2026 Lucio Lelii - ISTI-CNR +// SPDX-License-Identifier: AGPL-3.0-or-later +// Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM. + +package it.cnr.isti.workflow.manager.llms.providers.ollama; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.nio.charset.StandardCharsets; +import java.util.List; + +import org.junit.jupiter.api.Test; +import org.springframework.web.reactive.function.client.WebClient; + +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpServer; + +import it.cnr.isti.workflow.manager.llms.providers.LLMProviderHttpException; + +/** + * The real HTTP round trip through {@link OllamaProtocolProvider}, which nothing exercised before + * the extraction ({@code InternalOllamaLLMProviderBodyTest} only ever pinned the request bodies). + * Confirms both that the move preserved behaviour, and that it gained what + * {@link it.cnr.isti.workflow.manager.llms.providers.AbstractHttpLLMProvider} adds: a 4xx response + * now carries its body, where the old hand-written {@code onStatus} only ever mapped 5xx. + */ +class InternalOllamaLLMProviderHttpTest { + + private InternalOllamaLLMProvider providerFor(HttpServer server) { + String baseUrl = "http://127.0.0.1:" + server.getAddress().getPort(); + return new InternalOllamaLLMProvider(WebClient.builder(), "server-key", baseUrl); + } + + @Test + void generateReturnsTheParsedResponseField() throws IOException { + HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + server.createContext("/generate", exchange -> { + assertEquals("Bearer server-key", exchange.getRequestHeaders().getFirst("Authorization")); + writeJson(exchange, 200, "{\"response\":\"hello\"}"); + }); + server.start(); + try { + assertEquals("hello", providerFor(server).generate("m", "hi")); + } finally { + server.stop(0); + } + } + + @Test + void chatReturnsTheParsedMessageContent() throws IOException { + HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + server.createContext("/chat", exchange -> + writeJson(exchange, 200, "{\"message\":{\"role\":\"assistant\",\"content\":\"hi there\"}}")); + server.start(); + try { + String result = providerFor(server).chat("m", + List.of(new it.cnr.isti.workflow.manager.llms.ChatMessage( + it.cnr.isti.workflow.manager.llms.ChatMessage.Role.USER, "hi"))); + assertEquals("hi there", result); + } finally { + server.stop(0); + } + } + + @Test + void getRegisteredModelsParsesTheTagsResponse() throws IOException { + HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + server.createContext("/tags", exchange -> + writeJson(exchange, 200, "{\"models\":[{\"name\":\"llama3.2:3b\"},{\"name\":\"qwen3:8b\"}]}")); + server.start(); + try { + assertEquals(List.of("llama3.2:3b", "qwen3:8b"), providerFor(server).getRegisteredModels()); + } finally { + server.stop(0); + } + } + + @Test + void aFourHundredResponseNowCarriesItsBodyWhereTheOldCodeDroppedIt() throws IOException { + HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + server.createContext("/generate", exchange -> + writeJson(exchange, 404, "{\"error\":\"model 'nonexistent' not found\"}")); + server.start(); + try { + LLMProviderHttpException error = assertThrows(LLMProviderHttpException.class, + () -> providerFor(server).generate("nonexistent", "hi")); + assertEquals(404, error.statusCode()); + assertTrue(error.responseBody().contains("nonexistent")); + } finally { + server.stop(0); + } + } + + @Test + void aFiveHundredResponseStillCarriesItsBodyAsBefore() throws IOException { + HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + server.createContext("/chat", exchange -> writeJson(exchange, 500, "internal error")); + server.start(); + try { + LLMProviderHttpException error = assertThrows(LLMProviderHttpException.class, + () -> providerFor(server).chat("m", List.of(new it.cnr.isti.workflow.manager.llms.ChatMessage( + it.cnr.isti.workflow.manager.llms.ChatMessage.Role.USER, "hi")))); + assertEquals(500, error.statusCode()); + assertTrue(error.responseBody().contains("internal error")); + } finally { + server.stop(0); + } + } + + private static void writeJson(HttpExchange exchange, int status, String body) throws IOException { + byte[] bytes = body.getBytes(StandardCharsets.UTF_8); + exchange.getResponseHeaders().set("Content-Type", "application/json"); + exchange.sendResponseHeaders(status, bytes.length); + try (var output = exchange.getResponseBody()) { + output.write(bytes); + } + } + +} diff --git a/src/test/java/it/cnr/isti/workflow/manager/llms/providers/ollama/RemoteOllamaProviderTest.java b/src/test/java/it/cnr/isti/workflow/manager/llms/providers/ollama/RemoteOllamaProviderTest.java new file mode 100644 index 0000000..727eaa1 --- /dev/null +++ b/src/test/java/it/cnr/isti/workflow/manager/llms/providers/ollama/RemoteOllamaProviderTest.java @@ -0,0 +1,79 @@ +// SPDX-FileCopyrightText: 2025-2026 Lucio Lelii - ISTI-CNR +// SPDX-License-Identifier: AGPL-3.0-or-later +// Attribution term under AGPL-3.0 section 7(b): see LICENSE-ADDENDUM. + +package it.cnr.isti.workflow.manager.llms.providers.ollama; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; +import org.springframework.web.reactive.function.client.WebClient; + +import it.cnr.isti.workflow.manager.llms.ProviderCredential; +import it.cnr.isti.workflow.manager.llms.providers.OutboundEndpointGuard; + +class RemoteOllamaProviderTest { + + // Permissive: this class is about the provider's own logic, not the guard's - see + // OutboundEndpointGuardTest for that. + private final RemoteOllamaProvider provider = new RemoteOllamaProvider(WebClient.builder(), + new OutboundEndpointGuard(true, "")); + + @Test + void identifiesItselfAsRemoteOllama() { + assertEquals("RemoteOllama", provider.getName()); + } + + @Test + void requiresBothAnEndpointAndACredentialUnlikeTheInternalInstance() { + assertTrue(provider.requiresEndpoint()); + assertTrue(provider.requiresAuthorization()); + } + + @Test + void cannotListModelsSinceTheEditorHasNoCredentialToAskWith() { + assertFalse(provider.canListModels()); + assertTrue(provider.getRegisteredModels().isEmpty()); + } + + @Test + void resolvesTheUrlFromTheCredential() { + assertEquals("http://10.0.0.7:11434", + provider.baseUrl(new ProviderCredential("key", "http://10.0.0.7:11434"))); + } + + @Test + void refusesToCallWithNoEndpointOnTheCredential() { + assertThrows(IllegalArgumentException.class, () -> provider.baseUrl(new ProviderCredential("key", null))); + } + + @Test + void refusesToCallWithNoKeyOnTheCredential() { + // resolveApiKey is protected, not public - reachable directly since this test shares + // OllamaProtocolProvider's package, the same way the body tests reach buildGenerateBody. + assertThrows(IllegalArgumentException.class, + () -> provider.resolveApiKey(new ProviderCredential(null, "http://10.0.0.7:11434"))); + } + + @Test + void routesTheCredentialsEndpointThroughTheOutboundGuard() { + RemoteOllamaProvider guarded = new RemoteOllamaProvider(WebClient.builder(), + new OutboundEndpointGuard(false, "")); + + assertThrows(IllegalArgumentException.class, + () -> guarded.baseUrl(new ProviderCredential("key", "http://10.0.0.5:11434"))); + } + + @Test + void theStringOnlyPathFailsLoudlyRatherThanSilentlyMisbehaving() { + // The path an executor still on the deferred, endpoint-unaware wiring would use (plan + // step 11): ProviderCredential.ofValue(authorization) carries no endpoint. + IllegalArgumentException error = assertThrows(IllegalArgumentException.class, + () -> provider.generate("m", "hi", "bare-key-with-no-endpoint")); + assertTrue(error.getMessage().contains("endpoint")); + } + +}