From 33b82d161211b5a4c694afc4abbea639051804e8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 09:41:34 +0000 Subject: [PATCH 1/2] fix(build): make srcmorph/pom.xml parseable again main does not build at all -- not because of the unpublished 5.2.0 dependency, but because the POM is not well-formed XML: Non-parseable POM .../srcmorph/pom.xml: in comment after two dashes (--) next character must be > not f (position: END_TAG seen ... (--f... @54:73) XML forbids '--' inside a comment. The llama.version rationale I added in 5b4abeb spelled the CLI flags with their leading dashes (--flash-attn, --tensor-read-lazy, --lazy-mode) and used an em-dash-as-two-hyphens, four occurrences in one comment block. Maven cannot even read the project, so every goal fails before dependency resolution is reached -- which is why this hid behind the expected 5.2.0 failure. The flags are named without the leading dashes and the punctuation dash is replaced by a semicolon; no wording is lost. Verified: all four reactor POMs now have no '--' inside any comment (checked by regex over every block, not by eye), the file parses under a strict XML parser, and 'mvn validate' exits 0. My own miss: 5b4abeb edited a POM and I pushed it without running a single Maven command against it. --- srcmorph/pom.xml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/srcmorph/pom.xml b/srcmorph/pom.xml index 686a400..c65519b 100644 --- a/srcmorph/pom.xml +++ b/srcmorph/pom.xml @@ -51,13 +51,13 @@ SPDX-License-Identifier: Apache-2.0 InferenceParameters, ModelParameters, ChatResponse/Timings/Pair, ChatResponseParser, ReasoningFormat) is untouched by 5.1.0. 5.2.0 is a BREAKING bump this module needs, not an optional one: the binding removed - ModelParameters.enableFlashAttn()/ModelFlag.FLASH_ATTN (--flash-attn takes a mandatory + ModelParameters.enableFlashAttn()/ModelFlag.FLASH_ATTN (the flash-attn flag takes a mandatory on|off|auto value since llama.cpp b10273, so the valueless flag emitted a broken argv) and renamed setTensorReadLazy/TensorReadLazyMode to setLazyMode/LazyMode (llama.cpp - b10731 renamed --tensor-read-lazy to --lazy-mode with no alias). The provider calls + b10731 renamed the tensor-read-lazy flag to lazy-mode with no alias). The provider calls both, so it cannot compile against 5.1.0 any more. This deliberately names the RELEASE, not 5.2.0-SNAPSHOT: a snapshot would need a - entry for the Central snapshot channel (this repo declares none -- the + entry for the Central snapshot channel (this repo declares none; the distributionManagement/snapshotRepository below is deploy-only) AND would block every srcmorph release, since Central rejects a release with a -SNAPSHOT dependency. The build therefore stays red until net.ladenthin:llama:5.2.0 is published, and goes green From 33a55f2ac96c94fa8c7ffb67fe7aaa72b9fe06f6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 09:25:46 +0000 Subject: [PATCH 2/2] feat!: send enable_thinking only when it was actually set chatTemplateEnableThinking was a plain boolean defaulting to true, so every run put enable_thinking into the chat-template kwargs -- including runs whose chat template has never heard of the kwarg. llama.cpp's Jinja layer has been moving such unknown kwargs from "silently ignored" toward "warned about", and the only way to stop that noise was to set the knob to false, which means something else entirely. The obvious phrasing of the fix -- "send it only when it differs from the template's default" -- is not implementable: srcmorph would have to parse and evaluate the template to know that default, which is exactly the work it delegates to the binding. So the rule is "send it only when the user actually set it", and that needs a tri-state. AiGenerationConfig, AiModelDefinition and LlamaCppJniConfig (plus its builder) now carry @Nullable Boolean; DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING is null (unset) instead of true; the two isChatTemplateEnableThinking() getters become getChatTemplateEnableThinking() to match the wrapper type. true and false are both still forwarded verbatim -- unset is the only value that omits the kwarg. The kwarg assembly moves out of model() into a package-private buildChatTemplateKwargs(), which is what makes this testable at all: model() loads a GGUF, so nothing about the kwargs could be pinned without one. Five model-free guards now cover it, including the true case -- a later "send it only when false" shortcut would swallow a configured value, and now fails instead. reasoningEffort is deliberately left alone. It also defaults to a value that is sent on every run, but unlike enable_thinking it has an escape that means exactly "unset" (the empty string), so it is a documented choice rather than the same defect; the test javadoc records that rather than quietly widening the change. BREAKING CHANGE: chatTemplateEnableThinking is @Nullable Boolean, not boolean, and its getters are renamed to getChatTemplateEnableThinking(). A configuration that never mentioned the knob keeps working and simply stops sending the kwarg; one that set it explicitly is unaffected. --- CHANGELOG.md | 35 +++++- TODO.md | 18 --- srcmorph-maven-plugin/README.md | 2 +- .../srcmorph/config/AiGenerationConfig.java | 43 ++++--- .../srcmorph/config/AiModelDefinition.java | 17 +-- .../config/AiModelDefinitionSupport.java | 2 +- .../LlamaCppJniAiGenerationProvider.java | 41 +++++-- .../srcmorph/provider/LlamaCppJniConfig.java | 22 ++-- .../provider/LlamaCppJniConfigFactory.java | 2 +- .../config/AiModelDefinitionSupportTest.java | 4 +- .../LlamaCppJniAiGenerationProviderTest.java | 106 ++++++++++++++++++ 11 files changed, 229 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40d5dbe..c5f43b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,9 +43,38 @@ The release procedure (prompt template and step-by-step instructions) lives in [ native** produces `Failed to parse model parameters` at load time and nothing earlier catches it. The knob sweep did — it is what surfaced the mismatch here. -- **`net.ladenthin:llama` 5.1.0 → 5.2.0-SNAPSHOT.** Deliberately a snapshot: the binding change this - release depends on is not yet published. Building srcmorph therefore requires that snapshot to be - resolvable, and CI stays red until it is — recorded here so nobody mistakes it for a regression. +- **`net.ladenthin:llama` 5.1.0 → 5.2.0.** Deliberately the *release* version, not `5.2.0-SNAPSHOT`: + this repo declares no `` element (the `distributionManagement/snapshotRepository` is + deploy-only), so the Central snapshot channel is not resolvable here at all — and a `-SNAPSHOT` + dependency on `main` would block every srcmorph release, since Central rejects one. The build + therefore stays red until `net.ladenthin:llama:5.2.0` is published and goes green with no further + edit the moment it is — recorded here so nobody mistakes it for a regression. + +- **BREAKING: `chatTemplateEnableThinking` is a tri-state; the `enable_thinking` kwarg is only sent + when it was actually set.** It used to be a plain `boolean` defaulting to `true`, so every run put + `enable_thinking` into the chat-template kwargs -- including runs whose chat template has never + heard of the kwarg, which llama.cpp's Jinja layer has been moving from "silently ignored" toward + "warned about". The only way to stop that noise was to set the knob to `false`, which means + something else entirely. + + The obvious phrasing -- "send it only when it differs from the template's default" -- is not + implementable: srcmorph would have to parse and evaluate the template to know that default, which + is exactly the work it delegates to the binding. So the rule is "send it only when the user + actually set it". + + `AiGenerationConfig`, `AiModelDefinition` and `LlamaCppJniConfig` (and its builder) now carry + `@Nullable Boolean` instead of `boolean`, `DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING` is `null` + (unset) instead of `true`, and the two `isChatTemplateEnableThinking()` getters were renamed to + `getChatTemplateEnableThinking()` to match the wrapper type. `true` and `false` are both still + forwarded verbatim -- unset is the only value that omits the kwarg. A configuration that never + mentioned the knob keeps working and simply stops sending it; one that set it explicitly is + unaffected. + + Guarded by four model-free tests over the extracted `buildChatTemplateKwargs()` -- including the + `true` case, so a future "send it only when false" shortcut fails rather than silently swallowing + a configured value. + +--- ## [1.2.0] - 2026-09-01 diff --git a/TODO.md b/TODO.md index 347d20f..8234988 100644 --- a/TODO.md +++ b/TODO.md @@ -55,24 +55,6 @@ recorded in git history and `crossrepostatus.md`, not here. not dropping the idea. Deliberately out of scope for 1.2.0: it is a build-time question, not a correctness one. -- **`enable_thinking` is sent unconditionally, including at its own default.** - `LlamaCppJniAiGenerationProvider.model()` always puts `enable_thinking` into - `chatTemplateKwargs`, at whatever `chatTemplateEnableThinking` says — and its default is `true`. - A model whose chat template does not know the kwarg gets it anyway; llama.cpp's Jinja layer has - been moving such unknown kwargs from "ignored" toward "warned about", so a default run emits noise - that the user did not ask for and cannot switch off without setting the knob to a value that means - something else. - - The obvious phrasing of the fix -- "send it only when it differs from the template's default" -- - is **not implementable**: srcmorph cannot know a template's default without parsing and evaluating - the template, which is exactly the work it delegates to the binding. The implementable fix is - "send it only when the user actually set it", and that needs the config field to become a tri-state - (`Boolean` rather than `boolean`, `null` = unset), which changes `AiGenerationConfig`, - `AiModelDefinition`, `LlamaCppJniConfig` and its builder, plus the plugin's `@Parameter`. That is a - public-API change, so it belongs in a minor release with the deprecation story written out, not in - a patch. **Was announced during the 1.2.0 audit cycle and never landed** -- recorded here rather - than left as a claim in a chat log. - - **`srcmorph:calibrate` reports only through the log.** `CalibrateEngine` builds a `CalibrationReport` and `CalibrateMojo` prints it as `INFO` lines. There is no machine-readable output, so the numbers a calibration run produces (prefill / decode throughput, chars per token per diff --git a/srcmorph-maven-plugin/README.md b/srcmorph-maven-plugin/README.md index 6a17cee..09dc954 100644 --- a/srcmorph-maven-plugin/README.md +++ b/srcmorph-maven-plugin/README.md @@ -442,7 +442,7 @@ below are the shipped values (`AiGenerationConfig.DEFAULT_*`). | `lazyMode` | *(empty)* | On-demand reading of lazy-loadable tensors (`--tensor-read-lazy`): `off`, `auto` or `on`; empty leaves the default. Trades resident memory for disk reads and shortens model load time; **requires mmap** | | `mainGpu` | `-1` | Primary GPU index (`--main-gpu`); `-1` = leave default. Matters on multi-GPU hosts (e.g. a Vulkan build enumerates every GPU) | | `devices` | *(empty)* | Explicit device selection (`--device`), comma-separated backend device names (e.g. `Vulkan1`); takes precedence over `mainGpu` | -| `chatTemplateEnableThinking` | `true` | Enable the chat template's thinking mode | +| `chatTemplateEnableThinking` | *(unset)* | Chat-template thinking mode (`enable_thinking` kwarg): `true` or `false` is forwarded verbatim, unset omits the kwarg so the model's own template default applies. Set `false` for Gemma 4 to keep thought tokens out of stored output | | `reasoningEffort` | `low` | gpt-oss harmony reasoning effort (`low`/`medium`/`high`); empty omits the kwarg (e.g. for non-gpt-oss models) | | `reasoningBudgetTokens` | `-1` | Cap on harmony reasoning tokens (`-1` = unrestricted) | | `dryMultiplier` | `0.0` | DRY repetition-penalty multiplier (`0.0` = disabled); the other `dry*` knobs only apply when this is `> 0` | diff --git a/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiGenerationConfig.java b/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiGenerationConfig.java index 81cd0cc..fcf6de0 100644 --- a/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiGenerationConfig.java +++ b/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiGenerationConfig.java @@ -123,15 +123,24 @@ public AiGenerationConfig() { public static final float DEFAULT_REPEAT_PENALTY = 1.0f; /** - * Default setting for whether the model's chat-template thinking mode is enabled. + * Default setting for the model's chat-template thinking mode: {@code null}, meaning + * unset. * - *

When enabled, the model uses its own chat-template default for chain-of-thought - * reasoning. When disabled, {@link net.ladenthin.llama.parameters.ModelParameters#setChatTemplateKwargs} - * is called with {@code {"enable_thinking": "false"}} to suppress the thinking block at the - * Jinja template level — set to {@code false} for Gemma 4 definitions to prevent - * {@code <|channel>thought} tokens from leaking into stored output.

+ *

The value is a tri-state on purpose. While it was a plain {@code boolean} defaulting to + * {@code true}, every run put {@code enable_thinking} into + * {@link net.ladenthin.llama.parameters.ModelParameters#setChatTemplateKwargs} — including + * runs whose chat template has never heard of the kwarg, which llama.cpp's Jinja layer has been + * moving from "silently ignored" toward "warned about". There was no way to switch that noise + * off except by setting the knob to {@code false}, which means something else entirely.

+ * + *

"Send it only when it differs from the template's default" is not implementable here: + * srcmorph would have to parse and evaluate the template to know that default, which is exactly + * the work it delegates to the binding. So the rule is "send it only when the user actually set + * it": {@code null} omits the kwarg and the model's own chat-template default applies, while + * {@code true} and {@code false} are both forwarded verbatim. Set {@code false} for Gemma 4 + * definitions to keep {@code <|channel>thought} tokens out of stored output.

*/ - public static final boolean DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING = true; + public static final @Nullable Boolean DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING = null; /** * Default for whether llama.cpp prompt caching ({@code cache_prompt}) is enabled. @@ -387,7 +396,7 @@ public AiGenerationConfig() { private float minP = DEFAULT_MIN_P; private float topNSigma = DEFAULT_TOP_N_SIGMA; private float repeatPenalty = DEFAULT_REPEAT_PENALTY; - private boolean chatTemplateEnableThinking = DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING; + private @Nullable Boolean chatTemplateEnableThinking = DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING; private boolean cachePrompt = DEFAULT_CACHE_PROMPT; private boolean swaFull = DEFAULT_SWA_FULL; private int cacheReuse = DEFAULT_CACHE_REUSE; @@ -655,20 +664,26 @@ public void setTopNSigma(final float topNSigma) { } /** - * Returns whether the model's chat-template thinking mode is enabled. + * Returns the configured chat-template thinking mode, or {@code null} when it was never set. * - * @return {@code true} if chat-template thinking mode is enabled + * @return {@code true} or {@code false} when the knob was set, {@code null} when it is unset + * — in which case the {@code enable_thinking} kwarg is not sent at all and the + * model's own chat-template default applies + * @see #DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING */ - public boolean isChatTemplateEnableThinking() { + public @Nullable Boolean getChatTemplateEnableThinking() { return chatTemplateEnableThinking; } /** - * Sets whether the model's chat-template thinking mode is enabled. + * Sets the model's chat-template thinking mode. * - * @param chatTemplateEnableThinking {@code true} to enable chat-template thinking mode + * @param chatTemplateEnableThinking {@code true} or {@code false} to forward + * {@code enable_thinking} to the chat template, {@code null} to omit the kwarg entirely + * and leave the template's own default in place + * @see #DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING */ - public void setChatTemplateEnableThinking(final boolean chatTemplateEnableThinking) { + public void setChatTemplateEnableThinking(final @Nullable Boolean chatTemplateEnableThinking) { this.chatTemplateEnableThinking = chatTemplateEnableThinking; } diff --git a/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiModelDefinition.java b/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiModelDefinition.java index 3209963..474008e 100644 --- a/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiModelDefinition.java +++ b/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiModelDefinition.java @@ -51,7 +51,8 @@ public AiModelDefinition() { private float minP = AiGenerationConfig.DEFAULT_MIN_P; private float topNSigma = AiGenerationConfig.DEFAULT_TOP_N_SIGMA; private float repeatPenalty = AiGenerationConfig.DEFAULT_REPEAT_PENALTY; - private boolean chatTemplateEnableThinking = AiGenerationConfig.DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING; + private @Nullable Boolean chatTemplateEnableThinking = + AiGenerationConfig.DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING; private boolean cachePrompt = AiGenerationConfig.DEFAULT_CACHE_PROMPT; private boolean swaFull = AiGenerationConfig.DEFAULT_SWA_FULL; private int cacheReuse = AiGenerationConfig.DEFAULT_CACHE_REUSE; @@ -324,24 +325,26 @@ public void setTopNSigma(final float topNSigma) { } /** - * Returns whether the model's chat-template thinking mode is enabled. + * Returns this definition's chat-template thinking mode, or {@code null} when it was never set. * - * @return {@code true} to keep thinking enabled via the model's chat-template default; + * @return {@code true} or {@code false} when the knob was set, {@code null} when it is unset + * — the kwarg is then not sent and the model's own chat-template default applies; * defaults to {@link AiGenerationConfig#DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING} */ - public boolean isChatTemplateEnableThinking() { + public @Nullable Boolean getChatTemplateEnableThinking() { return chatTemplateEnableThinking; } /** - * Sets whether the model's chat-template thinking mode is enabled. + * Sets this definition's chat-template thinking mode. * * @param chatTemplateEnableThinking {@code false} passes * {@code enable_thinking=false} to * {@link net.ladenthin.llama.parameters.ModelParameters#setChatTemplateKwargs} to suppress - * chain-of-thought reasoning at the Jinja template level + * chain-of-thought reasoning at the Jinja template level, {@code true} passes it as + * {@code true}, and {@code null} omits the kwarg entirely */ - public void setChatTemplateEnableThinking(final boolean chatTemplateEnableThinking) { + public void setChatTemplateEnableThinking(final @Nullable Boolean chatTemplateEnableThinking) { this.chatTemplateEnableThinking = chatTemplateEnableThinking; } diff --git a/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiModelDefinitionSupport.java b/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiModelDefinitionSupport.java index 629aeaf..cc0e692 100644 --- a/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiModelDefinitionSupport.java +++ b/srcmorph/src/main/java/net/ladenthin/srcmorph/config/AiModelDefinitionSupport.java @@ -118,7 +118,7 @@ private static AiGenerationConfig toConfig(final AiModelDefinition definition) { config.setTopNSigma(definition.getTopNSigma()); config.setRepeatPenalty(definition.getRepeatPenalty()); config.setStopStrings(definition.getStopStrings()); - config.setChatTemplateEnableThinking(definition.isChatTemplateEnableThinking()); + config.setChatTemplateEnableThinking(definition.getChatTemplateEnableThinking()); config.setCachePrompt(definition.isCachePrompt()); config.setSwaFull(definition.isSwaFull()); config.setCacheReuse(definition.getCacheReuse()); diff --git a/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProvider.java b/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProvider.java index a85fec0..328fa16 100644 --- a/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProvider.java +++ b/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProvider.java @@ -90,18 +90,43 @@ public LlamaCppJniAiGenerationProvider(final LlamaCppJniConfig config, final AiP this.promptSupport = Objects.requireNonNull(promptSupport, "promptSupport"); } + /** + * Builds the chat-template kwargs passed to + * {@link net.ladenthin.llama.parameters.ModelParameters#setChatTemplateKwargs}. + * + *

Both kwargs are opt-in: an entry is written only when the user actually configured it, so a + * chat template that has never heard of the kwarg is not handed it. That matters because + * llama.cpp's Jinja layer has been moving unknown kwargs from "silently ignored" toward "warned + * about" -- while {@code enable_thinking} was a plain {@code boolean} defaulting to {@code true}, + * every run sent it, and the only way to stop the noise was to set the knob to {@code false}, + * which means something else entirely.

+ * + *

Package-private so it can be pinned without loading a GGUF; {@link #model()} is the only + * production caller.

+ * + * @return the kwargs to send; empty when neither knob is configured + */ + Map buildChatTemplateKwargs() { + final Map chatTemplateKwargs = + new HashMap<>(compatibilityHelper.hashMapCapacityFor(CHAT_TEMPLATE_KWARG_COUNT)); + // Qwen-style thinking. Unset (null) omits the kwarg so the model's own template default applies. + final Boolean enableThinking = config.chatTemplateEnableThinking(); + if (enableThinking != null) { + chatTemplateKwargs.put(ENABLE_THINKING_KWARG, String.valueOf(enableThinking.booleanValue())); + } + // gpt-oss honors reasoning_effort; non-gpt-oss chat templates ignore it. An empty + // configured value omits the kwarg so the model's own template default applies. + if (!compatibilityHelper.isBlank(config.reasoningEffort())) { + chatTemplateKwargs.put(REASONING_EFFORT_KWARG, config.reasoningEffort()); + } + return chatTemplateKwargs; + } + /** Loads the GGUF model on first use and caches it for subsequent calls. */ private LlamaModel model() { LlamaModel current = model; if (current == null) { - final Map chatTemplateKwargs = - new HashMap<>(compatibilityHelper.hashMapCapacityFor(CHAT_TEMPLATE_KWARG_COUNT)); - chatTemplateKwargs.put(ENABLE_THINKING_KWARG, String.valueOf(config.chatTemplateEnableThinking())); - // gpt-oss honors reasoning_effort; non-gpt-oss chat templates ignore it. An empty - // configured value omits the kwarg so the model's own template default applies. - if (!compatibilityHelper.isBlank(config.reasoningEffort())) { - chatTemplateKwargs.put(REASONING_EFFORT_KWARG, config.reasoningEffort()); - } + final Map chatTemplateKwargs = buildChatTemplateKwargs(); final ModelParameters modelParameters = new ModelParameters() .setModel(config.modelPath()) .setCtxSize(config.contextSize()) diff --git a/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniConfig.java b/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniConfig.java index d7732a7..858742b 100644 --- a/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniConfig.java +++ b/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniConfig.java @@ -10,6 +10,7 @@ import lombok.ToString; import net.ladenthin.srcmorph.config.AiGenerationConfig; import net.ladenthin.srcmorph.support.ConvertToRecord; +import org.jspecify.annotations.Nullable; /** * Immutable configuration for the llama.cpp JNI provider. @@ -37,7 +38,7 @@ public final class LlamaCppJniConfig { private final float minP; private final float topNSigma; private final float repeatPenalty; - private final boolean chatTemplateEnableThinking; + private final @Nullable Boolean chatTemplateEnableThinking; private final boolean cachePrompt; private final boolean swaFull; private final int cacheReuse; @@ -224,11 +225,14 @@ public float repeatPenalty() { } /** - * Returns whether chat-template thinking mode is enabled. + * Returns the chat-template thinking mode, or {@code null} when it was never set. * - * @return {@code true} when chat-template thinking mode is enabled + * @return {@code true} or {@code false} when the knob was set, {@code null} when it is unset + * — the provider then omits the {@code enable_thinking} kwarg entirely and the + * model's own chat-template default applies + * @see AiGenerationConfig#DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING */ - public boolean chatTemplateEnableThinking() { + public @Nullable Boolean chatTemplateEnableThinking() { return chatTemplateEnableThinking; } @@ -489,7 +493,8 @@ public static final class Builder { private float minP = AiGenerationConfig.DEFAULT_MIN_P; private float topNSigma = AiGenerationConfig.DEFAULT_TOP_N_SIGMA; private float repeatPenalty = AiGenerationConfig.DEFAULT_REPEAT_PENALTY; - private boolean chatTemplateEnableThinking = AiGenerationConfig.DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING; + private @Nullable Boolean chatTemplateEnableThinking = + AiGenerationConfig.DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING; private boolean cachePrompt = AiGenerationConfig.DEFAULT_CACHE_PROMPT; private boolean swaFull = AiGenerationConfig.DEFAULT_SWA_FULL; private int cacheReuse = AiGenerationConfig.DEFAULT_CACHE_REUSE; @@ -621,12 +626,13 @@ public Builder repeatPenalty(final float repeatPenalty) { } /** - * Sets whether chat-template thinking mode is enabled. + * Sets the chat-template thinking mode. * - * @param chatTemplateEnableThinking whether chat-template thinking mode is enabled + * @param chatTemplateEnableThinking {@code true} or {@code false} to forward + * {@code enable_thinking} to the chat template, {@code null} to omit the kwarg * @return this builder */ - public Builder chatTemplateEnableThinking(final boolean chatTemplateEnableThinking) { + public Builder chatTemplateEnableThinking(final @Nullable Boolean chatTemplateEnableThinking) { this.chatTemplateEnableThinking = chatTemplateEnableThinking; return this; } diff --git a/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniConfigFactory.java b/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniConfigFactory.java index 8ea9d49..7126b29 100644 --- a/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniConfigFactory.java +++ b/srcmorph/src/main/java/net/ladenthin/srcmorph/provider/LlamaCppJniConfigFactory.java @@ -43,7 +43,7 @@ public static LlamaCppJniConfig fromGenerationConfig(final AiGenerationConfig co .minP(config.getMinP()) .topNSigma(config.getTopNSigma()) .repeatPenalty(config.getRepeatPenalty()) - .chatTemplateEnableThinking(config.isChatTemplateEnableThinking()) + .chatTemplateEnableThinking(config.getChatTemplateEnableThinking()) .cachePrompt(config.isCachePrompt()) .swaFull(config.isSwaFull()) .cacheReuse(config.getCacheReuse()) diff --git a/srcmorph/src/test/java/net/ladenthin/srcmorph/config/AiModelDefinitionSupportTest.java b/srcmorph/src/test/java/net/ladenthin/srcmorph/config/AiModelDefinitionSupportTest.java index 22099a7..949eb57 100644 --- a/srcmorph/src/test/java/net/ladenthin/srcmorph/config/AiModelDefinitionSupportTest.java +++ b/srcmorph/src/test/java/net/ladenthin/srcmorph/config/AiModelDefinitionSupportTest.java @@ -98,7 +98,7 @@ public void getConfig_knownKey_propagatesAllFields() { assertThat(config.getThreads(), is(equalTo(4))); assertThat(config.getCharsPerToken(), is(equalTo(3))); assertThat(config.isWarnOnTrim(), is(false)); - assertThat(config.isChatTemplateEnableThinking(), is(false)); + assertThat(config.getChatTemplateEnableThinking(), is(false)); // topP/topK/repeatPenalty/stopStrings are propagated too — kills the void-call mutants // that would drop those setter calls from toConfig(). assertThat(config.getTopP(), is(equalTo(0.55f))); @@ -164,7 +164,7 @@ public void getConfig_defaultValues_matchAiGenerationConfigDefaults() { assertThat(config.getCharsPerToken(), is(equalTo(AiGenerationConfig.DEFAULT_CHARS_PER_TOKEN))); assertThat(config.getMaxInputChars(), is(equalTo(AiGenerationConfig.DEFAULT_MAX_INPUT_CHARS))); assertThat(config.isWarnOnTrim(), is(AiGenerationConfig.DEFAULT_WARN_ON_TRIM)); - assertThat(config.isChatTemplateEnableThinking(), is(AiGenerationConfig.DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING)); + assertThat(config.getChatTemplateEnableThinking(), is(AiGenerationConfig.DEFAULT_CHAT_TEMPLATE_ENABLE_THINKING)); assertThat(config.isCachePrompt(), is(AiGenerationConfig.DEFAULT_CACHE_PROMPT)); assertThat(config.isSwaFull(), is(AiGenerationConfig.DEFAULT_SWA_FULL)); assertThat(config.getCacheReuse(), is(equalTo(AiGenerationConfig.DEFAULT_CACHE_REUSE))); diff --git a/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProviderTest.java b/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProviderTest.java index bbf4fa2..d1c9c5e 100644 --- a/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProviderTest.java +++ b/srcmorph/src/test/java/net/ladenthin/srcmorph/provider/LlamaCppJniAiGenerationProviderTest.java @@ -15,6 +15,7 @@ import ch.qos.logback.core.read.ListAppender; import java.nio.file.Paths; import java.util.Collections; +import java.util.Map; import net.ladenthin.llama.args.CacheType; import net.ladenthin.llama.args.LazyMode; import net.ladenthin.llama.value.ChatChoice; @@ -44,6 +45,12 @@ public class LlamaCppJniAiGenerationProviderTest { /** JSON key of the repeat-penalty window, quoted for the same reason. */ private static final String PARAM_REPEAT_LAST_N = "\"repeat_last_n\""; + /** Chat-template kwarg key for Qwen-style thinking, as the provider spells it. */ + private static final String KWARG_ENABLE_THINKING = "enable_thinking"; + + /** Chat-template kwarg key for the gpt-oss reasoning-effort level. */ + private static final String KWARG_REASONING_EFFORT = "reasoning_effort"; + private static final AiMdHeader HEADER = new AiMdHeader( "Test.java", AiMdHeaderCodec.HEADER_VERSION_1_0, @@ -396,5 +403,104 @@ public void logPromptCacheReuse_debugDisabled_saysNothing() { } } + // + + /** + * The point of the tri-state. While {@code chatTemplateEnableThinking} was a plain + * {@code boolean} defaulting to {@code true}, every run handed {@code enable_thinking} to the + * chat template -- including templates that have never heard of it, which llama.cpp's Jinja + * layer has been moving from "silently ignored" toward "warned about". Unset must now mean + * "say nothing", so the template's own default applies. + * + *

The sibling kwarg is deliberately not asserted away here: {@code reasoningEffort} does + * default to {@code "low"} and is therefore still sent by a default run, but unlike + * {@code enable_thinking} it has an escape that means exactly "unset" -- the empty string -- so a + * non-gpt-oss user can switch it off without picking a value that says something else. That is a + * documented choice, not the same defect; the next test pins that escape.

+ */ + @Test + public void buildChatTemplateKwargs_defaultConfig_doesNotSendEnableThinking() { + // arrange + final LlamaCppJniConfig defaults = + LlamaCppJniConfig.builder("/does/not/exist.gguf").build(); + + // act + final Map kwargs = providerWith(defaults).buildChatTemplateKwargs(); + + // assert + assertThat(kwargs.containsKey(KWARG_ENABLE_THINKING), is(false)); + } + + /** The other half of "unset means unset": a blank reasoning effort omits its kwarg too. */ + @Test + public void buildChatTemplateKwargs_blankReasoningEffort_sendsNoKwargAtAll() { + // arrange + final LlamaCppJniConfig blank = LlamaCppJniConfig.builder("/does/not/exist.gguf") + .reasoningEffort("") + .build(); + + // act + final Map kwargs = providerWith(blank).buildChatTemplateKwargs(); + + // assert + assertThat(kwargs.isEmpty(), is(true)); + } + + /** + * {@code true} is a real configured value, not a second spelling of "unset". A guard written as + * "send it only when false" would swallow it, so this pins the boundary from the other side -- + * the same shape as the {@code 0}-versus-{@code -1} penalty-window pair above. + */ + @Test + public void buildChatTemplateKwargs_thinkingSetToTrue_isStillSent() { + // arrange + final LlamaCppJniConfig enabled = LlamaCppJniConfig.builder("/does/not/exist.gguf") + .chatTemplateEnableThinking(Boolean.TRUE) + .build(); + + // act + final Map kwargs = providerWith(enabled).buildChatTemplateKwargs(); + + // assert + assertThat(kwargs.get(KWARG_ENABLE_THINKING), is("true")); + } + + /** The Gemma-4 case the knob exists for: suppress the thinking block at the Jinja level. */ + @Test + public void buildChatTemplateKwargs_thinkingSetToFalse_isSentAsFalse() { + // arrange + final LlamaCppJniConfig disabled = LlamaCppJniConfig.builder("/does/not/exist.gguf") + .chatTemplateEnableThinking(Boolean.FALSE) + .build(); + + // act + final Map kwargs = providerWith(disabled).buildChatTemplateKwargs(); + + // assert + assertThat(kwargs.get(KWARG_ENABLE_THINKING), is("false")); + } + + /** + * The second kwarg was already opt-in (blank omits it); pinned here so the extraction of + * {@code buildChatTemplateKwargs} out of {@code model()} cannot drop it unnoticed. + */ + @Test + public void buildChatTemplateKwargs_reasoningEffortConfigured_isSentAlongside() { + // arrange + final LlamaCppJniConfig configured = LlamaCppJniConfig.builder("/does/not/exist.gguf") + .reasoningEffort("high") + .chatTemplateEnableThinking(Boolean.FALSE) + .build(); + + // act + final Map kwargs = providerWith(configured).buildChatTemplateKwargs(); + + // assert + assertThat(kwargs.get(KWARG_REASONING_EFFORT), is("high")); + assertThat(kwargs.get(KWARG_ENABLE_THINKING), is("false")); + } + + // + // }