lelii
/
humainflow-service
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Ask Ollama whether a model can reason before telling it to 

Every Ollama call carried think:true unconditionally, on the belief - stated
in a comment - that a model without the capability would ignore it. It does
not: Ollama answers with 400 "gemma:7b" does not support thinking, so every
flow pointed at gemma, llama3 or any other non-reasoning model failed outright.

Resolved per model rather than per flow, in two steps so that neither an old
Ollama nor one behind a proxy is left broken:

- /api/show reports a capability list, and "thinking" in it is the answer.
  Cached per endpoint and model, so it is asked once, not per call.
- When it cannot be asked - no capabilities field, /show unreachable - the
  flag is sent anyway and the rejection itself is the answer: the identical
  call is repeated without it and the result remembered. One wasted request
  per model per process, and none after that. Matched on the server's own
  wording, never the bare 400, so a misspelled model stays the error it is.

Off means the key is absent, not think:false - absence is the only state every
version of Ollama reads as "no opinion".

ModelParameters gains "reasoning" to overrule that per-model answer: ON sends
it whatever the model is and lets an incapable one fail rather than quietly
answering without it, OFF never sends it - the case detection cannot guess,
a model that does reason but where the tokens are not worth it. Empty, which
is what every existing flow has, is the per-model default.

An enum and not a boolean because the editor collapses a boolean to true or
false as soon as its group is opened (schema-driven-fields, next[key] =
rawValue === true), which would have turned reasoning off for the models that
have it. It follows MCPAgentUploadKind, lenient @JsonCreator included, since
a select nobody chose from posts "" and an enum cannot be coerced from that.

THINKING is deliberately outside LLMProvider.supportedParameters()'s default,
which is no longer allOf: only the Ollama protocol has the switch, and a
provider inheriting a claim to it would leave the run log silent about a
setting that reached nothing.

The MCP agent path carries the same setting but cannot detect anything - the
bridge is not Ollama and exposes no capability endpoint - so there it is
whatever the flow says, defaulting to on as before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Lucio Lelii 2026-09-22 09:48:14 +02:00
parent 10de89f172
commit 085ab55413
13 changed files with 648 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/main/java/it/cnr/isti/workflow/manager/llms/ModelParameter.java
View File

@ -5,14 +5,21 @@
package it.cnr.isti.workflow.manager.llms;
/**
* The sampling knobs a caller can set. Named here as an enum so a provider can declare which ones
* it honours, and the executor can report the rest on the run rather than letting a value the user
* set quietly do nothing.
* The knobs a caller can set on a model call. Named here as an enum so a provider can declare which
* ones it honours, and the executor can report the rest on the run rather than letting a value the
* user set quietly do nothing.
*/
public enum ModelParameter {
TEMPERATURE,
TOP_P,
TOP_K,
MAX_TOKENS,
SEED
SEED,
/**
* Not a sampling knob like the five above but a capability switch, which is why
* {@code LLMProvider.supportedParameters()} no longer defaults to "all of them": a provider has
* to claim this one deliberately. Only the Ollama protocol has it today.
*/
THINKING
}

27
src/main/java/it/cnr/isti/workflow/manager/llms/ModelParameters.java
View File

@ -20,7 +20,12 @@ import jakarta.validation.constraints.Min;
import lombok.Builder;
/**
* Optional sampling parameters for a model call.
* Optional parameters for a model call: how to sample from it, and whether to let it reason.
*
* <p>{@code reasoning} is the odd one out - a capability switch rather than a sampling knob, and
* the only one whose empty state triggers a per-model decision rather than simply sending nothing.
* It lives here anyway for the same reason the rest do: the MCP blocks need it and have no
* descriptor.
*
* <p>Every component is nullable and independent: null means "leave it to the provider", so an
* unset object - or an object with only one field set - behaves exactly as before this existed.
@ -76,7 +81,24 @@ public record ModelParameters(
@UiDescription("Fixes the randomness, so the same inputs give the same answer. Needed to tell a real change from model noise.")
@JsonProperty(required = false)
@DefaultsWhenEmpty
Long seed) {
Long seed,
/**
* Empty means "decide per model": the provider asks the model whether it can reason before
* sending anything, so a reasoning model keeps its trace out of the answer and one that
* cannot - gemma, llama3 - is simply not asked to. That is what every existing flow gets,
* and it is what repairs them: the flag used to be sent unconditionally, which Ollama
* answers with {@code 400 "model does not support thinking"}.
*
* <p>Set it only to overrule that per-model answer - see {@link ReasoningMode} for what the
* two constants mean and why this is not a boolean.
*/
@UiOrder(60)
@UiLabel("Reasoning")
@UiDescription("Leave empty to use reasoning on the models that support it. OFF skips it; ON requires it, and fails on a model that cannot reason.")
@JsonProperty(required = false)
@DefaultsWhenEmpty
ReasoningMode reasoning) {
/** Which knobs this object actually sets. Empty when it asks for nothing. */
@JsonIgnore
@ -87,6 +109,7 @@ public record ModelParameters(
if (topK != null) declared.add(ModelParameter.TOP_K);
if (maxTokens != null) declared.add(ModelParameter.MAX_TOKENS);
if (seed != null) declared.add(ModelParameter.SEED);
if (reasoning != null) declared.add(ModelParameter.THINKING);
return declared;
}

41
src/main/java/it/cnr/isti/workflow/manager/llms/ReasoningMode.java Normal file
View File

@ -0,0 +1,41 @@
// SPDX-FileCopyrightText: 2025-2026 Lucio Lelii <lucio.lelii@isti.cnr.it> - 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;
import com.fasterxml.jackson.annotation.JsonCreator;
/**
* Whether a model is asked to reason, when the answer is not left to the model itself.
*
* <p>Two constants and a null rather than a boolean, because the useful default is a third state:
* unset means "decide per model", which is what repairs the flows this was added for and what every
* existing one keeps. A boolean cannot carry that here - the editor collapses one to true or false
* the moment its group is opened, which would turn reasoning off for models that have it without
* anyone asking.
*/
public enum ReasoningMode {
/**
* Send the flag whatever the model is. A demand, not a stronger default: a model that cannot
* reason fails the call rather than quietly answering without it.
*/
ON,
/**
* Never send it. For a model that reasons but where the tokens and latency are not worth it -
* the one case detection cannot guess, since the capability is there.
*/
OFF;
/**
* A select nobody chose from posts "" rather than leaving the field out, and an enum cannot be
* coerced from that - see {@code MCPAgentUploadKind.fromString}, which this follows. Blank is
* read as unset, which is exactly the per-model default.
*/
@JsonCreator
public static ReasoningMode fromString(String value) {
return value == null || value.isBlank() ? null : valueOf(value.trim().toUpperCase());
}
}

9
src/main/java/it/cnr/isti/workflow/manager/llms/providers/LLMProvider.java
View File

@ -119,12 +119,17 @@ public interface LLMProvider {
}
/**
* Which sampling parameters this provider actually applies. Everything, by default: a provider
* Which parameters this provider actually applies. Every sampling knob by default: a provider
* that maps none of them still behaves as it always has, and only one that knowingly leaves a
* knob out should narrow this.
*
* <p>{@link ModelParameter#THINKING} is deliberately not in the default set, which is why this
* is no longer {@code allOf}. It is a capability only the Ollama protocol has, and a provider
* inheriting a claim to it would mean the run log stays silent about a reasoning switch that
* never reached anything - the exact failure {@code supportedParameters} exists to prevent.
*/
default Set<ModelParameter> supportedParameters() {
return EnumSet.allOf(ModelParameter.class);
return EnumSet.complementOf(EnumSet.of(ModelParameter.THINKING));
}
/**

230
src/main/java/it/cnr/isti/workflow/manager/llms/providers/ollama/OllamaProtocolProvider.java
View File

@ -5,10 +5,14 @@
package it.cnr.isti.workflow.manager.llms.providers.ollama;
import java.util.ArrayList;
import java.util.EnumSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -17,18 +21,22 @@ 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.ModelParameter;
import it.cnr.isti.workflow.manager.llms.ModelParameters;
import it.cnr.isti.workflow.manager.llms.ProviderCredential;
import it.cnr.isti.workflow.manager.llms.ReasoningMode;
import it.cnr.isti.workflow.manager.llms.ToolCall;
import it.cnr.isti.workflow.manager.llms.ToolChatResult;
import it.cnr.isti.workflow.manager.llms.ToolDefinition;
import it.cnr.isti.workflow.manager.llms.providers.AbstractHttpLLMProvider;
import it.cnr.isti.workflow.manager.llms.providers.LLMProviderHttpException;
import it.cnr.isti.workflow.manager.llms.providers.ollama.response.ChatResponse;
import it.cnr.isti.workflow.manager.llms.providers.ollama.response.ChatResponseMessage;
import it.cnr.isti.workflow.manager.llms.providers.ollama.response.ToolCallResponse;
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 it.cnr.isti.workflow.manager.llms.providers.ollama.response.ShowResponse;
import reactor.core.publisher.Mono;
/**
@ -48,8 +56,32 @@ import reactor.core.publisher.Mono;
public abstract class OllamaProtocolProvider extends AbstractHttpLLMProvider {
private static final Logger log = LoggerFactory.getLogger(OllamaProtocolProvider.class);
/** What Ollama calls the reasoning capability in the {@code /api/show} capability list. */
private static final String THINKING_CAPABILITY = "thinking";
/**
* The error Ollama answers a think:true it will not accept with, lowercased. Matched on the
* text because the status alone (400) is what a misspelled model returns too, and reacting to
* that one by silently retrying would hide it.
*/
private static final String THINKING_REJECTION = "does not support thinking";
private final ObjectMapper objectMapper = new ObjectMapper();
/**
* Whether an endpoint and model accept {@code think:true}, remembered so the question is asked
* once rather than per call. Keyed by both because the same model name on two Ollama instances
* can genuinely differ - one of them may be old enough not to validate the flag at all.
*
* <p>Unbounded on purpose: an entry is one boolean per model actually called, and the set of
* models a deployment calls is small and does not grow on its own. An instance lives as long as
* the application, so a model re-pulled with different capabilities keeps its old answer until
* restart - accepted, because the reasoning capability is a property of the architecture and
* does not change under the same tag in practice.
*/
private final Map<String, Boolean> thinkAccepted = new ConcurrentHashMap<>();
protected OllamaProtocolProvider(WebClient.Builder webClientBuilder) {
super(webClientBuilder);
}
@ -129,14 +161,11 @@ public abstract class OllamaProtocolProvider extends AbstractHttpLLMProvider {
ModelParameters parameters) {
Objects.requireNonNull(model, "model cannot be null");
Objects.requireNonNull(prompt, "prompt cannot be null");
Map<String, Object> body = buildGenerateBody(model, prompt, jsonResponse, parameters);
Mono<String> 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));
String baseUrl = baseUrl(credential);
String apiKey = resolveApiKey(credential);
return parseGenerateResponse(callDecidingOnThinking(baseUrl, apiKey, model, parameters,
think -> post(baseUrl, apiKey, "/generate", model,
buildGenerateBody(model, prompt, jsonResponse, parameters, think))));
}
@Override
@ -159,14 +188,159 @@ public abstract class OllamaProtocolProvider extends AbstractHttpLLMProvider {
ProviderCredential credential, ModelParameters parameters) {
Objects.requireNonNull(model, "model cannot be null");
Objects.requireNonNull(messages, "messages cannot be null");
Map<String, Object> body = buildChatBody(model, messages, parameters, tools);
Mono<String> request = withErrorHandling(clientFor(baseUrl(credential)).post()
.uri("/chat")
.header("Authorization", "Bearer " + resolveApiKey(credential))
String baseUrl = baseUrl(credential);
String apiKey = resolveApiKey(credential);
return callDecidingOnThinking(baseUrl, apiKey, model, parameters,
think -> post(baseUrl, apiKey, "/chat", model,
buildChatBody(model, messages, parameters, think, tools)));
}
/** The one POST both call paths go through, so the retry below re-sends an identical request. */
private String post(String baseUrl, String apiKey, String path, String model, Map<String, Object> body) {
Mono<String> request = withErrorHandling(clientFor(baseUrl).post()
.uri(path)
.header("Authorization", "Bearer " + apiKey)
.contentType(MediaType.APPLICATION_JSON)
.bodyValue(body)
.retrieve());
return blockingCall(request, "chat", model);
return blockingCall(request, path.substring(1), model);
}
/**
* The only provider family with a reasoning switch, so the only one that may claim it - see
* {@code LLMProvider.supportedParameters()} for why the default leaves it out.
*/
@Override
public Set<ModelParameter> supportedParameters() {
return EnumSet.allOf(ModelParameter.class);
}
/** One attempt at a call, with the reasoning flag either in the body or left out of it. */
@FunctionalInterface
private interface ThinkableCall {
String run(boolean think);
}
/**
* What to do with the reasoning flag on one call, and whether being wrong about it is
* recoverable.
*
* <p>The distinction that matters is between {@link #AUTO} and {@link #DEMANDED}: both send the
* flag, but only the first may quietly stop sending it. Asking for reasoning explicitly and
* getting an answer produced without it is the failure mode that setting it explicitly is meant
* to rule out.
*/
private enum ThinkDecision {
/** The model cannot reason, or the flow asked for it not to. */
OFF(false, false),
/** Nobody said either way and the model can reason: send it, and recover if we were wrong. */
AUTO(true, true),
/** The flow asked for reasoning: a model that cannot must fail, not silently do without. */
DEMANDED(true, false);
private final boolean send;
private final boolean recoverable;
ThinkDecision(boolean send, boolean recoverable) {
this.send = send;
this.recoverable = recoverable;
}
}
/**
* Runs a call with the reasoning flag resolved for this model, and - when the resolution was a
* guess rather than an answer - repeats it without the flag if the server rejects it.
*
* <p>The retry is the part that makes this work against any Ollama, including one too old to
* report capabilities and one behind a proxy that will not answer {@code /api/show}. It costs
* one rejected request per model per process, and nothing after that: the rejection is what
* teaches the cache. Nothing else is retried here - the test below matches the server's own
* wording, not the 400, because a misspelled model name is a 400 too and must stay one.
*/
private String callDecidingOnThinking(String baseUrl, String apiKey, String model,
ModelParameters parameters, ThinkableCall call) {
ThinkDecision decision = resolveThinking(baseUrl, apiKey, model, parameters);
try {
return call.run(decision.send);
} catch (RuntimeException error) {
if (!decision.recoverable || !isThinkingRejection(error)) {
throw error;
}
log.info("Model {} on {} does not support thinking: retrying without it, and not asking again", model,
baseUrl);
thinkAccepted.put(cacheKey(baseUrl, model), Boolean.FALSE);
return call.run(false);
}
}
/** Empty means "decide per model"; a value set on the flow is obeyed as written. */
private ThinkDecision resolveThinking(String baseUrl, String apiKey, String model, ModelParameters parameters) {
ReasoningMode requested = parameters == null ? null : parameters.reasoning();
if (requested == ReasoningMode.OFF) {
return ThinkDecision.OFF;
}
if (requested == ReasoningMode.ON) {
return ThinkDecision.DEMANDED;
}
return thinkAccepted.computeIfAbsent(cacheKey(baseUrl, model), key -> probeThinking(baseUrl, apiKey, model))
? ThinkDecision.AUTO
: ThinkDecision.OFF;
}
/**
* Asks the server what this model can do, rather than guessing from its name.
*
* <p>Optimistic when it cannot tell, in both of the ways it can fail to: a server old enough to
* have no {@code capabilities} field is also old enough not to validate {@code think}, so
* sending it there is exactly what this provider has always done; and a {@code /api/show} that
* errors outright says nothing about the model, so the call itself is left to find out - which
* the retry above then handles. Never throws: a diagnostic endpoint must not be able to fail a
* run that would otherwise have worked.
*/
private boolean probeThinking(String baseUrl, String apiKey, String model) {
try {
Mono<String> request = withErrorHandling(clientFor(baseUrl).post()
.uri("/show")
.header("Authorization", "Bearer " + apiKey)
.contentType(MediaType.APPLICATION_JSON)
.bodyValue(Map.of("model", model))
.retrieve());
String body = request.block();
if (body == null || body.isBlank()) {
return true;
}
List<String> capabilities = objectMapper.readValue(body, ShowResponse.class).getCapabilities();
if (capabilities == null) {
return true;
}
boolean canThink = capabilities.stream().anyMatch(THINKING_CAPABILITY::equalsIgnoreCase);
log.debug("Model {} on {} reports capabilities {}, thinking {}", model, baseUrl, capabilities,
canThink ? "enabled" : "not requested");
return canThink;
} catch (RuntimeException probeFailure) {
log.debug("Could not read the capabilities of {} from {} ({}): leaving the reasoning flag on and letting"
+ " the call itself say otherwise", model, baseUrl, probeFailure.toString());
return true;
}
}
/**
* Matched on the server's wording rather than the status, and walked down the cause chain
* because the timeout and retry operators wrap what they propagate.
*/
private static boolean isThinkingRejection(Throwable error) {
for (Throwable cause = error; cause != null && cause != cause.getCause(); cause = cause.getCause()) {
if (cause instanceof LLMProviderHttpException http
&& http.responseBody().toLowerCase(Locale.ROOT).contains(THINKING_REJECTION)) {
return true;
}
}
return false;
}
/** Both halves, because the same model name on two instances is not the same model. */
private static String cacheKey(String baseUrl, String model) {
return baseUrl + '\u0000' + model;
}
/**
@ -206,7 +380,8 @@ public abstract class OllamaProtocolProvider extends AbstractHttpLLMProvider {
}
/** Package-private so the request body can be asserted without a server. The defaults it carries are load-bearing. */
Map<String, Object> buildGenerateBody(String model, String prompt, boolean jsonResponse, ModelParameters parameters) {
Map<String, Object> buildGenerateBody(String model, String prompt, boolean jsonResponse, ModelParameters parameters,
boolean think) {
Map<String, Object> bodyMap = new LinkedHashMap<>();
bodyMap.put("model", model);
bodyMap.put("prompt", prompt);
@ -216,8 +391,14 @@ public abstract class OllamaProtocolProvider extends AbstractHttpLLMProvider {
// 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);
// to skip reasoning altogether).
//
// Omitted rather than sent as false when this model cannot reason: Ollama answers a
// think:true it does not accept with a 400, and only the flag's absence is what every
// version of it treats as "no opinion". The caller decides - see resolveThinking.
if (think) {
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
@ -239,25 +420,28 @@ public abstract class OllamaProtocolProvider extends AbstractHttpLLMProvider {
}
/** Mutable, unlike the Map.of it replaces: with nothing set the body is the one it always sent. */
Map<String, Object> buildChatBody(String model, List<ChatMessage> messages, ModelParameters parameters) {
return buildChatBody(model, messages, parameters, List.of());
Map<String, Object> buildChatBody(String model, List<ChatMessage> messages, ModelParameters parameters,
boolean think) {
return buildChatBody(model, messages, parameters, think, List.of());
}
/**
* The same body, plus the tools the model may call.
*
* <p>With an empty tool list this is byte for byte the body the three-argument overload has
* always sent, which is what keeps the text path - every existing caller - unchanged.
* <p>With an empty tool list this is byte for byte the body the shorter overload has always
* sent, which is what keeps the text path - every existing caller - unchanged.
*/
Map<String, Object> buildChatBody(String model, List<ChatMessage> messages, ModelParameters parameters,
List<ToolDefinition> tools) {
boolean think, List<ToolDefinition> tools) {
Map<String, Object> bodyMap = new LinkedHashMap<>();
bodyMap.put("model", model);
bodyMap.put("messages", messages.stream().map(OllamaProtocolProvider::toOllamaMessage).toList());
bodyMap.put("stream", false);
// See buildGenerateBody: keeps reasoning out of message.content without asking the model to
// reason less.
bodyMap.put("think", true);
// reason less, and is left out entirely for a model that cannot reason.
if (think) {
bodyMap.put("think", true);
}
if (tools != null && !tools.isEmpty()) {
bodyMap.put("tools", tools.stream()
.map(tool -> Map.of("type", "function", "function", Map.of(

28
src/main/java/it/cnr/isti/workflow/manager/llms/providers/ollama/response/ShowResponse.java Normal file
View File

@ -0,0 +1,28 @@
// SPDX-FileCopyrightText: 2025-2026 Lucio Lelii <lucio.lelii@isti.cnr.it> - 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.response;
import java.util.List;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import lombok.Data;
import lombok.NoArgsConstructor;
/**
* What {@code /api/show} says about one model. Only {@code capabilities} is read - the response
* also carries the modelfile, the template and the full parameter dump, none of which we want.
*
* <p>The list names what the model can do: {@code completion}, {@code tools}, {@code thinking},
* {@code vision}, {@code insert}, {@code embedding}. Null when the server is older than the field,
* which is not the same as an empty list and must not be read as "can do nothing" - see
* {@code OllamaProtocolProvider.probeThinking}.
*/
@JsonIgnoreProperties(ignoreUnknown = true)
@Data
@NoArgsConstructor
public class ShowResponse {
private List<String> capabilities;
}

18
src/main/java/it/cnr/isti/workflow/manager/mcp/MCPAgentService.java
View File

@ -38,6 +38,7 @@ import tools.jackson.databind.ObjectMapper;
import it.cnr.isti.workflow.manager.commons.CircuitBreaker;
import it.cnr.isti.workflow.manager.llms.ModelParameters;
import it.cnr.isti.workflow.manager.llms.ReasoningMode;
import it.cnr.isti.workflow.manager.commons.Throwables;
import reactor.core.publisher.Mono;
@ -214,7 +215,16 @@ public class MCPAgentService {
// result (see InternalOllamaLLMProvider.buildGenerateBody), so a block's output isn't the
// model's raw chain-of-thought. Whether the bridge forwards this to its own Ollama call is
// outside this codebase, same as the options below - if it doesn't, this has no effect.
llmProvider.put("think", true);
//
// Unlike the direct Ollama path this cannot ask the model what it can do: the bridge is not
// Ollama and exposes no capability endpoint, so the flag is what the flow says it is and
// "empty" keeps the behaviour this has always had. A flow pointed at a model that cannot
// reason turns it off here by hand - which is why the setting exists as well as the
// detection. Sent only when on, because the flag's absence is the only thing every version
// reads as "no opinion".
if ((parameters == null ? null : parameters.reasoning()) != ReasoningMode.OFF) {
llmProvider.put("think", true);
}
if (!ModelParameters.isEmpty(parameters)) {
Map<String, Object> options = new LinkedHashMap<>();
if (parameters.temperature() != null) options.put("temperature", parameters.temperature());
@ -222,7 +232,11 @@ public class MCPAgentService {
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());
llmProvider.put("options", options);
// "think" alone declares parameters without declaring any option: an empty map here
// would be a shape the bridge has never been sent.
if (!options.isEmpty()) {
llmProvider.put("options", options);
}
}
request.put("llm_provider", llmProvider);
if (mcpServers != null && !mcpServers.isEmpty()) {

2
src/test/java/it/cnr/isti/workflow/manager/executions/executors/blocks/LLMToolLoopTest.java
View File

@ -164,7 +164,7 @@ class LLMToolLoopTest {
private static LLMDescriptor descriptorWithMaxTokens(int maxTokens) {
return new LLMDescriptor("Scripted", "test-model",
new it.cnr.isti.workflow.manager.llms.ModelParameters(null, null, null, maxTokens, null));
new it.cnr.isti.workflow.manager.llms.ModelParameters(null, null, null, maxTokens, null, null));
}
private static ObjectNode argumentsWithJsonLookingContent() {

2
src/test/java/it/cnr/isti/workflow/manager/llms/LLMProviderCatalogServiceTest.java
View File

@ -64,7 +64,7 @@ class LLMProviderCatalogServiceTest {
Map<String, LLMProviderMetadata> byName = catalog.list().stream()
.collect(java.util.stream.Collectors.toMap(LLMProviderMetadata::name, metadata -> metadata));
assertEquals(List.of("MAX_TOKENS", "SEED", "TEMPERATURE", "TOP_K", "TOP_P"),
assertEquals(List.of("MAX_TOKENS", "SEED", "TEMPERATURE", "THINKING", "TOP_K", "TOP_P"),
byName.get("Everything").supportedParameters(), "sorted, so the payload is stable");
assertEquals(List.of("TEMPERATURE", "TOP_K"), byName.get("NoSeed").supportedParameters());
assertTrue(byName.get("Everything").supportsTools());

29
src/test/java/it/cnr/isti/workflow/manager/llms/ModelParametersTest.java
View File

@ -6,6 +6,7 @@ package it.cnr.isti.workflow.manager.llms;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.util.Set;
@ -43,14 +44,38 @@ class ModelParametersTest {
}
@Test
void serialisesOnlyTheFiveParametersAndNothingElse() throws Exception {
void anEmptyReasoningSelectionMeansPerModelRatherThanARejectedUpdate() throws Exception {
// A select nobody chose from posts "" instead of leaving the field out, and an enum cannot
// be coerced from that: the whole block update would come back as a bad request naming
// "reasoning". Blank has to land on the per-model default, which is also the safe one.
ModelParameters parameters = new ObjectMapper()
.readValue("{\"reasoning\":\"\"}", ModelParameters.class);
assertNull(parameters.reasoning());
assertTrue(parameters.isEmpty());
}
@Test
void reasoningIsAThirdStateAndNotAFlagThatDefaultsToOff() {
// The distinction the editor cannot express with a checkbox, and the reason this is an enum:
// "not chosen" has to survive as its own value, or opening the parameter group would turn
// reasoning off for every model that has it.
assertTrue(ModelParameters.builder().build().isEmpty());
assertEquals(Set.of(ModelParameter.THINKING),
ModelParameters.builder().reasoning(ReasoningMode.OFF).build().declared());
assertEquals(Set.of(ModelParameter.THINKING),
ModelParameters.builder().reasoning(ReasoningMode.ON).build().declared());
}
@Test
void serialisesOnlyTheDeclaredParametersAndNothingElse() throws Exception {
// isEmpty() is an is-prefixed no-arg boolean, which Jackson reads as a property: without
// @JsonIgnore an "empty" field appeared in every stored flow and every API payload, in a
// shape the published schema does not declare. Caught by looking at a real saved block.
String json = new ObjectMapper()
.writeValueAsString(ModelParameters.builder().temperature(0.0).seed(42L).build());
assertEquals(Set.of("temperature", "topP", "topK", "maxTokens", "seed"),
assertEquals(Set.of("temperature", "topP", "topK", "maxTokens", "seed", "reasoning"),
new ObjectMapper().readTree(json).propertyNames());
}
}

56
src/test/java/it/cnr/isti/workflow/manager/llms/providers/ollama/InternalOllamaLLMProviderBodyTest.java
View File

@ -37,13 +37,13 @@ class InternalOllamaLLMProviderBodyTest {
@Test
void nothingSetLeavesTheTextRequestExactlyAsItWas() {
// The text path has never sent options. If it started sending an empty map, or defaults it
// did not send before, every existing flow would quietly change behaviour. "think" is sent
// unconditionally regardless (see buildGenerateBody), so a thinking model's reasoning trace
// is never mixed into "response" - unaffected by models/versions that don't support it.
// did not send before, every existing flow would quietly change behaviour. "think" is what
// the caller resolved for this model (see buildGenerateBody), and with a model that can
// reason that is still true - so a thinking model's trace is never mixed into "response".
assertEquals(Map.of("model", "m", "prompt", "p", "stream", false, "think", true),
ollama.buildGenerateBody("m", "p", false, null));
ollama.buildGenerateBody("m", "p", false, null, true));
assertEquals(Map.of("model", "m", "prompt", "p", "stream", false, "think", true),
ollama.buildGenerateBody("m", "p", false, ModelParameters.builder().build()));
ollama.buildGenerateBody("m", "p", false, ModelParameters.builder().build(), true));
}
@Test
@ -51,7 +51,7 @@ class InternalOllamaLLMProviderBodyTest {
// temperature 0.1 and num_predict 4096 have been forced on the JSON path since before
// parameters existed, and the flow assistant's output depends on them. This test exists to
// fail if anyone "tidies them away" while making the values configurable.
Map<String, Object> body = ollama.buildGenerateBody("m", "p", true, null);
Map<String, Object> body = ollama.buildGenerateBody("m", "p", true, null, true);
assertEquals("json", body.get("format"));
assertEquals(Map.of("temperature", 0.1, "num_predict", 4096), body.get("options"));
@ -60,7 +60,7 @@ class InternalOllamaLLMProviderBodyTest {
@Test
void aSetValueOverridesOnlyItsOwnDefault() {
Map<String, Object> body = ollama.buildGenerateBody("m", "p", true,
ModelParameters.builder().temperature(0.9).build());
ModelParameters.builder().temperature(0.9).build(), true);
@SuppressWarnings("unchecked")
Map<String, Object> options = (Map<String, Object>) body.get("options");
@ -72,7 +72,7 @@ class InternalOllamaLLMProviderBodyTest {
@Test
void mapsEveryParameterToOllamasOwnNames() {
Map<String, Object> body = ollama.buildGenerateBody("m", "p", false, ModelParameters.builder()
.temperature(0.2).topP(0.8).topK(40).maxTokens(512).seed(7L).build());
.temperature(0.2).topP(0.8).topK(40).maxTokens(512).seed(7L).build(), true);
assertEquals(Map.of("temperature", 0.2, "top_p", 0.8, "top_k", 40, "num_predict", 512, "seed", 7L),
body.get("options"));
@ -82,7 +82,7 @@ class InternalOllamaLLMProviderBodyTest {
void sendsOnlyTheParametersThatWereSet() {
// A partially filled object must not fill the gaps with invented values.
Map<String, Object> body = ollama.buildGenerateBody("m", "p", false,
ModelParameters.builder().seed(3L).build());
ModelParameters.builder().seed(3L).build(), true);
assertEquals(Map.of("seed", 3L), body.get("options"));
}
@ -91,7 +91,7 @@ class InternalOllamaLLMProviderBodyTest {
void theChatBodyIsUnchangedWithNothingSet() {
List<ChatMessage> messages = List.of(new ChatMessage(ChatMessage.Role.USER, "hi"));
Map<String, Object> body = ollama.buildChatBody("m", messages, null);
Map<String, Object> body = ollama.buildChatBody("m", messages, null, true);
assertEquals(4, body.size());
assertEquals(Boolean.TRUE, body.get("think"));
@ -99,11 +99,31 @@ class InternalOllamaLLMProviderBodyTest {
assertEquals(List.of(Map.of("role", "user", "content", "hi")), body.get("messages"));
}
@Test
void aModelThatCannotReasonGetsNoThinkKeyAtAll() {
// Not think:false. Ollama answers think:true on a model without the capability with a 400,
// and the key's absence is the only state every version of it reads as "no opinion" - so
// the off state has to be an omission, not a false.
Map<String, Object> body = ollama.buildGenerateBody("m", "p", false, null, false);
assertEquals(Map.of("model", "m", "prompt", "p", "stream", false), body);
assertFalse(body.containsKey("think"));
}
@Test
void theChatBodyAlsoLeavesThinkOutForAModelThatCannotReason() {
Map<String, Object> body = ollama.buildChatBody("m",
List.of(new ChatMessage(ChatMessage.Role.USER, "hi")), null, false);
assertFalse(body.containsKey("think"));
assertEquals(3, body.size());
}
@Test
void theChatBodyCarriesTheParametersWhenThereAreSome() {
Map<String, Object> body = ollama.buildChatBody("m",
List.of(new ChatMessage(ChatMessage.Role.USER, "hi")),
ModelParameters.builder().temperature(0.0).build());
ModelParameters.builder().temperature(0.0).build(), true);
assertEquals(Map.of("temperature", 0.0), body.get("options"));
}
@ -111,12 +131,12 @@ class InternalOllamaLLMProviderBodyTest {
@Test
void anEmptyToolListLeavesTheChatBodyExactlyAsItWas() {
// The overload with tools is what every chat call now goes through, so with none declared it
// has to produce the body the three-argument one always did - otherwise adding tool support
// would change every existing flow that never asked for a tool.
// has to produce the body the shorter one always did - otherwise adding tool support would
// change every existing flow that never asked for a tool.
List<ChatMessage> messages = List.of(new ChatMessage(ChatMessage.Role.USER, "hi"));
assertEquals(ollama.buildChatBody("m", messages, null),
ollama.buildChatBody("m", messages, null, List.of()));
assertEquals(ollama.buildChatBody("m", messages, null, true),
ollama.buildChatBody("m", messages, null, true, List.of()));
}
@Test
@ -125,7 +145,7 @@ class InternalOllamaLLMProviderBodyTest {
schema.put("type", "object");
Map<String, Object> body = ollama.buildChatBody("m",
List.of(new ChatMessage(ChatMessage.Role.USER, "hi")), null,
List.of(new ChatMessage(ChatMessage.Role.USER, "hi")), null, true,
List.of(new ToolDefinition("write_file", "writes a file", schema)));
assertEquals(List.of(Map.of("type", "function", "function", Map.of(
@ -140,7 +160,7 @@ class InternalOllamaLLMProviderBodyTest {
Map<String, Object> body = ollama.buildChatBody("m",
List.of(ChatMessage.assistantToolCalls("on it", List.of(ToolCall.of(0, "write_file", arguments)))),
null, List.of());
null, true, List.of());
assertEquals(List.of(Map.of(
"role", "assistant",
@ -154,7 +174,7 @@ class InternalOllamaLLMProviderBodyTest {
// Which one Ollama reads depends on its version, and picking wrong fails silently: the model
// just sees a result it cannot attribute to its call.
Map<String, Object> body = ollama.buildChatBody("m",
List.of(ChatMessage.toolResult("call_0", "write_file", "ok")), null, List.of());
List.of(ChatMessage.toolResult("call_0", "write_file", "ok")), null, true, List.of());
assertEquals(List.of(Map.of(
"role", "tool",

222
src/test/java/it/cnr/isti/workflow/manager/llms/providers/ollama/OllamaThinkingSupportTest.java Normal file
View File

@ -0,0 +1,222 @@
// SPDX-FileCopyrightText: 2025-2026 Lucio Lelii <lucio.lelii@isti.cnr.it> - 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 java.io.IOException;
import java.net.InetSocketAddress;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
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.ModelParameters;
import it.cnr.isti.workflow.manager.llms.ProviderCredential;
import it.cnr.isti.workflow.manager.llms.ReasoningMode;
import it.cnr.isti.workflow.manager.llms.providers.LLMProviderHttpException;
/**
* Whether the reasoning flag is sent, against a stand-in Ollama.
*
* <p>The behaviour under test is why this needs a server at all: it is a conversation, not a body.
* Ollama rejects {@code think:true} on a model that cannot reason - gemma, llama3 - with a 400, so
* a flag that was previously sent unconditionally broke every flow pointed at one of them. The
* provider now asks {@code /api/show} first, and treats a rejection as the answer when asking was
* not possible.
*/
class OllamaThinkingSupportTest {
private HttpServer ollama;
private final List<String> generateBodies = new CopyOnWriteArrayList<>();
private final List<String> shownModels = new CopyOnWriteArrayList<>();
/** Set per test before the first call, so each can stage a different server. */
private volatile List<String> capabilities;
private volatile boolean showFails;
@BeforeEach
void startOllama() throws IOException {
capabilities = List.of("completion", "thinking");
showFails = false;
ollama = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0);
ollama.createContext("/api/show", exchange -> {
shownModels.add(body(exchange));
if (showFails) {
writeJson(exchange, 500, "{\"error\":\"nope\"}");
return;
}
writeJson(exchange, 200, capabilities == null
? "{\"model_info\":{}}"
: "{\"capabilities\":[" + capabilities.stream().map(c -> '"' + c + '"').reduce((a, b) -> a + "," + b)
.orElse("") + "]}");
});
ollama.createContext("/api/generate", exchange -> {
String requestBody = body(exchange);
generateBodies.add(requestBody);
// Copied from a real Ollama, quoted model name and all, rather than paraphrased: the
// provider matches on the wording, so this string is the contract. Keying off the bare
// 400 instead would be caught by aRejectionThatIsNotAboutThinkingIsLeftAlone below.
if (requestBody.contains("\"think\"")) {
writeJson(exchange, 400, "{\"error\":\"\\\"gemma:7b\\\" does not support thinking\"}");
return;
}
writeJson(exchange, 200, "{\"response\":\"answered\"}");
});
ollama.start();
}
@AfterEach
void stopOllama() {
ollama.stop(0);
}
private InternalOllamaLLMProvider provider() {
return new InternalOllamaLLMProvider(WebClient.builder(), "test-key",
"http://127.0.0.1:" + ollama.getAddress().getPort() + "/api");
}
@Test
void aModelWithoutTheThinkingCapabilityIsNeverAskedToReason() {
capabilities = List.of("completion", "tools");
assertEquals("answered", provider().generate("gemma:7b", "hi"));
assertEquals(1, generateBodies.size());
assertFalse(generateBodies.get(0).contains("think"),
"a model that cannot reason must not be sent the flag at all");
}
@Test
void aModelThatCanReasonStillGetsTheFlag() {
capabilities = List.of("completion", "tools", "thinking");
assertEquals("answered", provider().generate("qwen3:8b", "hi"));
assertTrue(generateBodies.get(0).contains("\"think\":true"),
"a model that reports the capability must still be asked to reason");
}
@Test
void aServerThatCannotBeAskedIsRecoveredFromByTheRejectionItself() {
// The case /api/show cannot cover: an Ollama behind a proxy, or one too old to report
// capabilities. The first call is spent finding out, and the answer still comes back.
showFails = true;
assertEquals("answered", provider().generate("gemma:7b", "hi"));
assertEquals(2, generateBodies.size(), "one rejected attempt, then the same call without the flag");
assertTrue(generateBodies.get(0).contains("\"think\":true"));
assertFalse(generateBodies.get(1).contains("think"));
}
@Test
void aServerTooOldToReportCapabilitiesIsTreatedTheSameWay() {
// No "capabilities" field at all is not an empty capability list: such a version does not
// validate think either, so the flag is sent and the rejection - if any - teaches us.
capabilities = null;
assertEquals("answered", provider().generate("gemma:7b", "hi"));
assertEquals(2, generateBodies.size());
}
@Test
void theRejectionIsRememberedSoOnlyOneCallIsEverWasted() {
showFails = true;
InternalOllamaLLMProvider provider = provider();
assertEquals("answered", provider.generate("gemma:7b", "hi"));
assertEquals("answered", provider.generate("gemma:7b", "again"));
assertEquals("answered", provider.generate("gemma:7b", "and again"));
assertEquals(4, generateBodies.size(), "the first call pays for the discovery; the rest do not");
assertTrue(generateBodies.get(0).contains("\"think\":true"));
assertFalse(generateBodies.get(1).contains("think"));
assertFalse(generateBodies.get(2).contains("think"));
assertFalse(generateBodies.get(3).contains("think"));
}
@Test
void theCapabilityIsAskedForOncePerModelRatherThanPerCall() {
capabilities = List.of("completion");
InternalOllamaLLMProvider provider = provider();
provider.generate("gemma:7b", "hi");
provider.generate("gemma:7b", "again");
assertEquals(1, shownModels.size());
assertTrue(shownModels.get(0).contains("gemma:7b"));
}
@Test
void reasoningTurnedOffSkipsEvenAskingTheServer() {
// Nothing to discover: the flow has already said no, so the capability call is wasted work.
capabilities = List.of("completion", "thinking");
assertEquals("answered", provider().generate("qwen3:8b", "hi", (ProviderCredential) null,
ModelParameters.builder().reasoning(ReasoningMode.OFF).build()));
assertTrue(shownModels.isEmpty());
assertFalse(generateBodies.get(0).contains("think"));
}
@Test
void reasoningDemandedExplicitlyFailsRatherThanQuietlyRunningWithoutIt() {
// The difference between "on" and "unset": asking for reasoning and getting an answer
// produced without it is the outcome setting it explicitly exists to rule out.
capabilities = List.of("completion");
LLMProviderHttpException error = assertThrows(LLMProviderHttpException.class,
() -> provider().generate("gemma:7b", "hi", (ProviderCredential) null,
ModelParameters.builder().reasoning(ReasoningMode.ON).build()));
assertEquals(400, error.statusCode());
assertEquals(1, generateBodies.size(), "a demand must not be downgraded by the retry");
assertTrue(shownModels.isEmpty(), "an explicit value needs no detection");
}
@Test
void aRejectionThatIsNotAboutThinkingIsLeftAlone() throws IOException {
// A misspelled model name is a 400 too. Retrying that one without the flag would turn a
// clear error into a second identical failure, and hide which of the two it was.
ollama.removeContext("/api/generate");
ollama.createContext("/api/generate", exchange -> {
generateBodies.add(body(exchange));
writeJson(exchange, 400, "{\"error\":\"model 'qwen4' not found\"}");
});
capabilities = List.of("completion", "thinking");
LLMProviderHttpException error = assertThrows(LLMProviderHttpException.class,
() -> provider().generate("qwen4", "hi"));
assertTrue(error.responseBody().contains("not found"));
assertEquals(1, generateBodies.size(), "only a thinking rejection is retried");
}
private static String body(HttpExchange exchange) throws IOException {
return new String(exchange.getRequestBody().readAllBytes(), StandardCharsets.UTF_8);
}
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);
}
}
}

27
src/test/java/it/cnr/isti/workflow/manager/mcp/MCPAgentServiceTest.java
View File

@ -26,6 +26,7 @@ import com.sun.net.httpserver.HttpExchange;
import com.sun.net.httpserver.HttpServer;
import it.cnr.isti.workflow.manager.llms.ModelParameters;
import it.cnr.isti.workflow.manager.llms.ReasoningMode;
import org.junit.jupiter.api.io.TempDir;
import org.springframework.core.io.DefaultResourceLoader;
import org.springframework.web.reactive.function.client.WebClient;
@ -275,9 +276,8 @@ class MCPAgentServiceTest {
void openSessionRequestIsUnchangedWithNoParameters() throws Exception {
// The bridge is an external service with no contract in this repo. With nothing set in
// ModelParameters, "options" stays absent, so a bridge that knows nothing about options is
// unaffected by their existence. "think" is the one exception: it is sent unconditionally
// (not gated by ModelParameters) so a thinking model's reasoning trace never ends up mixed
// into the block's output, the same way InternalOllamaLLMProvider always asks for it too.
// unaffected by their existence. "think" is the one exception: unset means on, so a
// thinking model's reasoning trace never ends up mixed into the block's output.
Map<String, Object> request = serviceWithNoServers()
.buildOpenSessionRequest("m", List.of(), Map.of(), null);
@ -286,6 +286,27 @@ class MCPAgentServiceTest {
assertFalse(request.containsKey("max_steps"));
}
@Test
void reasoningTurnedOffLeavesTheFlagOutEntirely() throws Exception {
// This path cannot ask the bridge what the model can do, so turning it off by hand is the
// only remedy for a model that has no reasoning - and the flag has to be absent, not false,
// because absence is what every Ollama version reads as "no opinion".
Map<String, Object> request = serviceWithNoServers().buildOpenSessionRequest("m", List.of(), Map.of(),
ModelParameters.builder().reasoning(ReasoningMode.OFF).build());
assertEquals(Map.of("provider", "ollama", "model", "m"), request.get("llm_provider"));
}
@Test
void reasoningOnItsOwnDoesNotInventAnEmptyOptionsMap() throws Exception {
// "think" declares parameters without declaring any option. An empty "options" would be a
// shape the bridge has never been sent.
Map<String, Object> request = serviceWithNoServers().buildOpenSessionRequest("m", List.of(), Map.of(),
ModelParameters.builder().reasoning(ReasoningMode.ON).build());
assertEquals(Map.of("provider", "ollama", "model", "m", "think", true), request.get("llm_provider"));
}
@Test
void openSessionRequestCarriesTheParametersUnderTheProvider() throws Exception {