Merge pull request #32 from darth-veitcher/feat/disable-thinking-toggle

feat(ollama): add disable-thinking toggle for both providers

CHANGELOG.md

@@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 - README: What-is-this, Install, and Quickstart sections
 - `LLMProvider` protocol (`comfydv._llm`) — a shared adapter boundary so ComfyUI LLM nodes work with any backend that implements it, starting with `OllamaProvider`. Structured output now goes through `pydantic-ai` (ADR-007), superseding the hand-rolled Ollama tool-calling approach.
 - **Chat Completion** now accepts an optional `image` input for vision-capable models (VLMs): wire a ComfyUI `IMAGE` and the connected model can describe or reason about it. Works identically on both backends (Ollama multimodal models; llama.cpp launched with `--mmproj`), and composes with structured output and multi-turn history. Images are carried on `Message.images` and translated to each backend's native shape (Ollama's flat `images` array, llama.cpp's OpenAI `image_url` parts, pydantic-ai `BinaryContent` on the structured path) — ADR-008, extending ADR-007's adapter pattern to a second input modality. Text-only workflows are unchanged when no image is wired.
+- **Ollama Option — Disable Thinking** node: turn off (or explicitly re-enable) a "thinking"-capable model's chain-of-thought reasoning. Chains into the same composable `OLLAMA_OPTIONS` socket every other `OllamaOption*` node uses, but works for both backends — each `LLMProvider` implementation pops the `think` key back out and translates it to its own wire shape (Ollama: a top-level `think` field; llama.cpp: `chat_template_kwargs`/`reasoning_effort` request-body fields, not live-verified — see ADR-010).
 
 ### Changed
 - **Breaking:** `OllamaChatCompletion` → `ChatCompletion`, `OllamaModelSelector` → `LLMModelSelector`, `OllamaLoadModel` → `LLMLoadModel`, `OllamaUnloadModel` → `LLMUnloadModel`, and the `OLLAMA_CLIENT` socket type → `LLM_CLIENT` — these nodes are now backend-generic. `OllamaClient` is unchanged by name but now outputs an `OllamaProvider` rather than a plain string; existing saved workflows using the old node/socket names need reconnecting (see `comfydv.ollama.MIGRATION_MAP` for the full old→new mapping).

project-management/ADRs/ADR-010-disable-thinking-toggle.md

@@ -0,0 +1,64 @@
+# ADR-010: `"think"` as an options-carried, per-provider-translated toggle
+
+## Status
+
+> Accepted
+
+_Date:_ 2026-07-25
+_Deciders:_ darth-veitcher
+
+---
+
+## Context
+
+ADR-009's investigation into structured-output reliability surfaced, as a side effect, how expensive a "thinking"-capable model's chain-of-thought reasoning is: on a real workflow, a single agent call could spend 5-7+ minutes and its entire token budget on reasoning before ever producing the requested response. Both Ollama and llama-server can turn this off, but neither exposes it through the generic `options` dict `ChatCompletion` already forwards — each has a completely different, incompatible wire shape:
+
+- **Ollama** — live-tested directly: `"think": false` must be a **top-level** field on `/api/chat` (and `/v1/chat/completions`). Nested inside `options` (`{"options": {"think": false}}`) it's silently ignored — confirmed live (`eval_count: 223` reasoning tokens burned vs. `eval_count: 2` with it top-level). One existing test (`test_multi_turn_receives_context`) already carried `options={"think": False}` in its docstring's stated intent; it was a no-op the whole time.
+- **llama.cpp** — not live-tested (no router-mode `llama-server` instance available; user explicitly chose doc-based research over waiting for one). Per `tools/server/README.md`: `chat_template_kwargs: {"enable_thinking": false}` (Qwen3-style HF chat-template convention) and/or `reasoning_effort: "none"` (a more model-agnostic OpenAI-style convention llama-server also accepts) — both as request-body fields on `/v1/chat/completions`, not nested in `options` either.
+
+Two shapes were considered for exposing this from comfydv:
+
+1. **A first-class `ChatCompletion` input + `LLMProvider` protocol parameter** — mirroring how `Message.images` crossed the provider boundary (ADR-008). Initially implemented this way.
+2. **A composable `OllamaOption*`-style node merging a `"think"` key into the existing `OLLAMA_OPTIONS` chain**, with each provider popping that one key back out and translating it before building its own request — proposed as a simplification once (1) was drafted, since every other tunable knob already flows through this exact composition pattern and a new top-level node parameter would be the only one that doesn't.
+
+## Decision
+
+Went with option 2. `OllamaOptionDisableThinking` (`src/comfydv/ollama.py`) is a new node, identical in shape to `OllamaOptionTemperature`/`OllamaOptionSeed`/etc.: `disable_thinking: BOOLEAN` (default `True`), merges `{"think": not disable_thinking}` into whatever `OLLAMA_OPTIONS` chain it's wired into. No `ChatCompletion` or `LLMProvider` protocol signature change.
+
+Both providers now start `chat()`/`chat_structured()` by popping `"think"` out of the incoming `options` dict (`_pop_think()`, `ollama_provider.py`, shared by both — a pure function, doesn't mutate the caller's dict) and translate it into their own shape before building the request:
+
+- `OllamaProvider`: sets `payload["think"]` at the top level (both `chat()`'s native `/api/chat` call and `chat_structured()`'s, per ADR-009's native-endpoint rewrite).
+- `LlamaCppProvider`: sets `chat_template_kwargs`/`reasoning_effort` — directly in its own hand-rolled `chat()` payload, and via `chat.py`'s `extra_body` (the same mechanism `options` itself uses) for `chat_structured()`, which still shares the pydantic-ai path per ADR-009.
+
+Despite living in `ollama.py` and following the `OllamaOption*` naming convention (matching every other option node in that module, all genuinely Ollama-native and untranslated for llama.cpp — see `LlamaCppProvider.chat()`'s own comment), `"think"` is the one key from that chain **both** providers recognize and translate; it isn't itself Ollama's native wire format, it's a comfydv-level convention that happens to reuse Ollama's own field name since Ollama's is the more literal of the two backends' conventions.
+
+## Consequences
+
+**Easier:**
+- One node works for both backends, reusing the exact composition pattern (`OLLAMA_OPTIONS` chaining into `ChatCompletion`'s `options` input) every other tunable parameter already uses — no new socket type, no `ChatCompletion.INPUT_TYPES` change, no `LLMProvider` protocol change.
+- Fixes an existing test's stated-but-unfulfilled intent for free: `test_multi_turn_receives_context` and `test_structured_output_against_unreliable_model_stays_schema_valid` already passed `options={"think": False}` and now it actually works.
+- Meaningfully faster for any thinking-capable model, and directly reduces the token-budget pressure ADR-009 had to fix around.
+
+**Harder / constrained:**
+- The llama.cpp translation is not live-verified — sourced from the server's documented request-body fields, not confirmed against a running `llama-server`. Verify against your own deployment before relying on it; a follow-up should close this gap once an instance is available (the user explicitly chose this tradeoff over waiting).
+- `"think"` living among genuinely-Ollama-native `OllamaOption*` nodes (which llama.cpp does *not* translate — see that class's own code comment) is a small naming/mental-model inconsistency: one key out of that whole chain is special-cased by both providers. Documented here and in `_pop_think()`'s own docstring so it doesn't read as an oversight later.
+
+**Debt introduced:**
+- None. No new dependency, no new socket type.
+
+## Considered Alternatives
+
+### Alternative A: First-class `ChatCompletion` input + protocol parameter (mirroring `Message.images`, ADR-008)
+
+**Why rejected:** Correct in principle (this is a cross-provider concern needing real translation, exactly like images), but heavier than necessary — a new node input plus a `LLMProvider.chat()`/`chat_structured()` signature change plus threading a new parameter through every call site, when the existing `options` dict composition already has a clean seam (`_pop_think`) for a value that needs per-provider translation before hitting the wire. Started implementing this way; reverted once the composable-option alternative was raised.
+
+### Alternative B: Separate provider-specific nodes (`OllamaOptionDisableThinking` / a llama.cpp-only equivalent)
+
+**Why rejected:** Splits one concept into two nodes for no real benefit — both backends' translation lives in code either way, so there's no cost to having one node recognize the same key on both.
+
+---
+
+## Links
+
+- Related ADRs: [ADR-007](ADR-007-llm-provider-adapter-pattern.md) (the `LLMProvider` boundary this operates within), [ADR-008](ADR-008-multimodal-image-input-across-llmprovider-boundary.md) (the pattern this ADR considered and didn't need — cross-provider concerns don't always require a protocol change), [ADR-009](ADR-009-native-structured-output-mode.md) (the investigation that surfaced how expensive unmanaged thinking is)
+- llama.cpp server docs (request-body fields, not live-verified): `tools/server/README.md` in `ggml-org/llama.cpp`

src/comfydv/__init__.py

@@ -14,6 +14,7 @@ from .ollama import (
     OllamaHeaderBearerToken,
     OllamaHeaderCustom,
     OllamaHistoryLength,
+    OllamaOptionDisableThinking,
     OllamaOptionExtraBody,
     OllamaOptionMaxTokens,
     OllamaOptionRepeatPenalty,
@@ -46,6 +47,7 @@ NODE_CLASS_MAPPINGS = {
     "OllamaOptionTopP": OllamaOptionTopP,
     "OllamaOptionTopK": OllamaOptionTopK,
     "OllamaOptionRepeatPenalty": OllamaOptionRepeatPenalty,
+    "OllamaOptionDisableThinking": OllamaOptionDisableThinking,
     "OllamaOptionExtraBody": OllamaOptionExtraBody,
     "OllamaDebugHistory": OllamaDebugHistory,
     "OllamaHistoryLength": OllamaHistoryLength,
@@ -72,6 +74,7 @@ NODE_DISPLAY_NAME_MAPPINGS = {
     "OllamaOptionTopP": "Ollama Option — Top P",
     "OllamaOptionTopK": "Ollama Option — Top K",
     "OllamaOptionRepeatPenalty": "Ollama Option — Repeat Penalty",
+    "OllamaOptionDisableThinking": "Ollama Option — Disable Thinking",
     "OllamaOptionExtraBody": "Ollama Option — Extra Body",
     "OllamaDebugHistory": "Ollama Debug History",
     "OllamaHistoryLength": "Ollama History Length",

src/comfydv/_llm/chat.py

@@ -138,6 +138,20 @@ async def chat_structured(
     lossily remapped onto pydantic-ai's own standardized ``ModelSettings``
     fields.
 
+    ADR-010: ``options`` may also carry a ``"think"`` key (bool), popped out
+    here rather than forwarded inside the nested ``options`` object —
+    llama-server's OpenAI-compatible endpoint doesn't recognize a literal
+    ``"think"`` key there. Translated to its own two documented
+    request-body toggles instead: ``chat_template_kwargs:
+    {"enable_thinking": ...}`` (Qwen3-style models) and, when disabling,
+    ``reasoning_effort: "none"`` (the more model-agnostic OpenAI convention
+    llama-server also honors) — both via ``extra_body`` the same way
+    ``options`` is. This provider only serves ``LlamaCppProvider`` — see
+    ``OllamaProvider``'s own hand-rolled ``chat_structured`` for why Ollama
+    needed a different mechanism entirely. Sourced from llama.cpp's server
+    docs, not live-verified against a running llama-server (no instance
+    available at implementation time) — verify against your own deployment.
+
     Retries up to ``max_retries`` times (clamped 0-5) on validation failure
     before raising ``RuntimeError``. Never returns a value that failed
     validation against ``schema``.
@@ -156,8 +170,20 @@ async def chat_structured(
     )
     history = _history_to_messages(messages)
     prompt = _user_prompt_content(messages[-1])
+    think = None
+    if options and "think" in options:
+        options = dict(options)
+        think = options.pop("think")
+        options = options or None
+    extra_body: dict = {}
+    if options:
+        extra_body["options"] = options
+    if think is not None:
+        extra_body["chat_template_kwargs"] = {"enable_thinking": think}
+        if not think:
+            extra_body["reasoning_effort"] = "none"
     model_settings: ModelSettings | None = (
-        {"extra_body": {"options": options}} if options else None
+        {"extra_body": extra_body} if extra_body else None
     )
 
     total_attempts = max(0, min(int(max_retries), 5)) + 1

src/comfydv/_llm/llamacpp_provider.py

@@ -18,7 +18,7 @@ import logging
 
 from pydantic import BaseModel
 
-from .ollama_provider import _TTLLRUCache, _cache_key, _get_json, _post_json
+from .ollama_provider import _TTLLRUCache, _cache_key, _get_json, _pop_think, _post_json
 from .provider import Message, ModelInfo, ModelStatus
 from .retry import RETRY_BACKOFF_SECS, next_seed
 
@@ -185,6 +185,7 @@ class LlamaCppProvider:
         max_retries: int = 2,
     ) -> str:
         payload_messages = [_to_openai_message(m) for m in messages]
+        options, think = _pop_think(options)
         total_attempts = max(0, min(int(max_retries), 5)) + 1
         response_text = ""
 
@@ -205,6 +206,13 @@ class LlamaCppProvider:
                 # handling consistent rather than silently special-casing
                 # one of them.
                 payload["options"] = options
+            if think is not None:
+                # ADR-010: llama-server's two documented reasoning toggles —
+                # sourced from server docs, not live-verified (no instance
+                # available at implementation time).
+                payload["chat_template_kwargs"] = {"enable_thinking": think}
+                if not think:
+                    payload["reasoning_effort"] = "none"
             if attempt > 1:
                 # Unlike the options-passthrough above, this IS the OpenAI
                 # spec's actual top-level "seed" field, so it takes effect
@@ -218,6 +226,7 @@ class LlamaCppProvider:
                 model,
                 payload_messages,
                 options or {},
+                think,
                 payload.get("seed"),
             )
             cached, hit = _CHAT_RESPONSE_CACHE.get(cache_key)

src/comfydv/_llm/ollama_provider.py

@@ -79,6 +79,29 @@ def _cache_key(*parts) -> str:
     return json.dumps(parts, sort_keys=True, default=str)
 
 
+def _pop_think(options: dict | None) -> tuple[dict | None, bool | None]:
+    """Split a ``"think"`` toggle out of a generic ``options`` dict.
+
+    ADR-010: ``OllamaOptionDisableThinking`` merges a ``"think": bool`` key
+    into the same composable ``OLLAMA_OPTIONS`` chain every other
+    ``OllamaOption*`` node feeds into ``ChatCompletion``'s ``options``
+    input — but unlike those (Ollama-native sampling params, passed through
+    verbatim), ``"think"`` needs real per-provider translation: neither
+    Ollama's native ``/api/chat`` nor llama-server's OpenAI-compatible
+    endpoint recognizes a literal ``"think"`` key nested inside their own
+    ``options``/sampling-params object, so every provider pops it out here
+    (or in ``LlamaCppProvider``'s own copy) before building its request.
+    Returns ``options`` with ``"think"`` removed (unchanged if absent, so a
+    falsy/empty result stays falsy) and the popped value, or ``None`` if the
+    caller didn't set it — never touches the caller's own dict in place.
+    """
+    if not options or "think" not in options:
+        return options, None
+    remaining = dict(options)
+    think = remaining.pop("think")
+    return (remaining or None), think
+
+
 _MODEL_LIST_CACHE = _TTLLRUCache(maxsize=32, ttl_seconds=20.0)
 _CHAT_RESPONSE_CACHE = _TTLLRUCache(maxsize=64, ttl_seconds=None)
 _CAPABILITY_CACHE = _TTLLRUCache(maxsize=32, ttl_seconds=300.0)
@@ -329,6 +352,7 @@ class OllamaProvider:
         # (FR-003); a turn with images keeps Ollama's native flat images
         # array (ADR-008 — no transform needed for /api/chat).
         payload_messages = [m.model_dump(exclude_none=True) for m in messages]
+        options, think = _pop_think(options)
         total_attempts = max(0, min(int(max_retries), 5)) + 1
         response_text = ""
         incomplete = False
@@ -345,6 +369,10 @@ class OllamaProvider:
             }
             if attempt_options:
                 payload["options"] = attempt_options
+            if think is not None:
+                # ADR-010: confirmed live this must be a top-level field —
+                # Ollama silently ignores "think" nested inside "options".
+                payload["think"] = think
 
             cache_key = _cache_key(
                 "chat",
@@ -353,6 +381,7 @@ class OllamaProvider:
                 model,
                 payload_messages,
                 attempt_options,
+                think,
             )
             cached, hit = _CHAT_RESPONSE_CACHE.get(cache_key)
             if hit:
@@ -427,6 +456,7 @@ class OllamaProvider:
 
         payload_messages = [m.model_dump(exclude_none=True) for m in messages]
         json_schema = schema.model_json_schema()
+        options, think = _pop_think(options)
         cache_key = _cache_key(
             "chat_structured",
             self.host,
@@ -435,6 +465,7 @@ class OllamaProvider:
             payload_messages,
             options or {},
             json_schema,
+            think,
         )
         cached, hit = _CHAT_RESPONSE_CACHE.get(cache_key)
         if hit:
@@ -457,6 +488,10 @@ class OllamaProvider:
             }
             if attempt_options:
                 payload["options"] = attempt_options
+            if think is not None:
+                # ADR-010: confirmed live this must be a top-level field —
+                # Ollama silently ignores "think" nested inside "options".
+                payload["think"] = think
 
             try:
                 result = await _post_json(

src/comfydv/_llm/provider.py

@@ -93,6 +93,14 @@ class LLMProvider(Protocol):
         attempt's text rather than raising if every retry comes back blank —
         this method has never validated its output, unlike
         ``chat_structured()``.
+
+        ADR-010: ``options`` may carry a ``"think"`` key (bool) to disable a
+        "thinking"-capable model's chain-of-thought reasoning — every
+        implementation pops it out of ``options`` and translates it to its
+        own wire shape (Ollama: a top-level ``think`` field; llama.cpp:
+        ``chat_template_kwargs``/``reasoning_effort`` in the request body),
+        since neither backend recognizes a literal ``"think"`` key nested
+        inside a generic options object.
         """
         ...
 
@@ -111,5 +119,8 @@ class LLMProvider(Protocol):
         truncated snippet of the last invalid response) if every retry is
         exhausted — never returns a value with a missing/blank required
         field.
+
+        ADR-010: see ``chat()`` — same ``options["think"]`` convention,
+        same per-provider translation.
         """
         ...

src/comfydv/ollama.py

@@ -851,6 +851,52 @@ class OllamaOptionRepeatPenalty:
         return (_merge_option(options, "repeat_penalty", repeat_penalty),)
 
 
+class OllamaOptionDisableThinking:
+    """Turn off (or explicitly re-enable) a "thinking"-capable model's
+    chain-of-thought reasoning (ADR-010).
+
+    Rides the same composable ``OLLAMA_OPTIONS`` chain as every other
+    ``OllamaOption*`` node, but unlike those (Ollama-native sampling
+    params passed through verbatim), the ``"think"`` key this node emits is
+    a comfydv-level convention: every ``LLMProvider`` implementation pops
+    it out of the merged ``options`` dict and translates it to its own
+    wire shape — Ollama's native top-level ``think`` field (confirmed live:
+    silently ignored if left nested in ``options``), or llama-server's
+    ``chat_template_kwargs``/``reasoning_effort`` request-body fields
+    (per llama.cpp's server docs — not live-verified). Works for both
+    backends from the same node.
+    """
+
+    @classmethod
+    def INPUT_TYPES(s):
+        return {
+            "required": {
+                "disable_thinking": (
+                    "BOOLEAN",
+                    {
+                        "default": True,
+                        "tooltip": (
+                            "On: skip chain-of-thought reasoning entirely "
+                            "— faster, and the model's whole token budget "
+                            "goes to the actual response. Off: explicitly "
+                            "re-enable thinking (only useful to override a "
+                            "server-side default)."
+                        ),
+                    },
+                ),
+            },
+            "optional": {"options": ("OLLAMA_OPTIONS",)},
+        }
+
+    RETURN_TYPES = ("OLLAMA_OPTIONS",)
+    RETURN_NAMES = ("options",)
+    FUNCTION = "set_disable_thinking"
+    CATEGORY = "dv/ollama/options"
+
+    def set_disable_thinking(self, disable_thinking, options=None):
+        return (_merge_option(options, "think", not disable_thinking),)
+
+
 class OllamaOptionExtraBody:
     @classmethod
     def INPUT_TYPES(s):

tests/test_llamacpp_provider.py

@@ -358,6 +358,33 @@ def test_chat_retry_seed_is_top_level_not_nested_in_options(monkeypatch):
     assert calls[2]["seed"] == 2
 
 
+def test_chat_disable_thinking_sets_chat_template_kwargs_and_reasoning_effort(
+    monkeypatch,
+):
+    """ADR-010: llama-server doesn't recognize a "think" key nested inside
+    "options" (that's an Ollama-native convention) — it needs its own two
+    documented request-body toggles instead, and "think" must not leak into
+    the nested options object llama-server actually does understand."""
+    captured = {}
+
+    async def fake_post(url, payload, *, timeout=120.0, headers=None):
+        captured.update(payload)
+        return {"choices": [{"message": {"content": "ok"}}]}
+
+    monkeypatch.setattr(provider_mod, "_post_json", fake_post)
+    _run_async(
+        LlamaCppProvider("http://localhost:8080").chat(
+            "gemma-3-4b",
+            [Message(role="user", content="hi")],
+            options={"temperature": 0.5, "think": False},
+        )
+    )
+
+    assert captured["chat_template_kwargs"] == {"enable_thinking": False}
+    assert captured["reasoning_effort"] == "none"
+    assert captured["options"] == {"temperature": 0.5}  # "think" popped out
+
+
 def test_chat_exhausted_retries_returns_blank_without_raising(monkeypatch):
     calls = {"n": 0}
 

tests/test_llm_chat_structured.py

@@ -223,6 +223,53 @@ def test_chat_structured_no_options_means_no_model_settings(monkeypatch):
     assert fake.calls[0][2] is None
 
 
+def test_chat_structured_disable_thinking_sets_chat_template_kwargs(monkeypatch):
+    """ADR-010: llama-server's two documented reasoning toggles, applied via
+    extra_body the same way options is — "think" must not leak into the
+    nested options.extra_body.options object llama-server's native sampling
+    params live in."""
+    fake = _FakeAgent([_Widget(name="a", count=1)])
+    monkeypatch.setattr(chat_mod, "_build_agent", lambda **kw: fake)
+
+    _run_async(
+        chat_mod.chat_structured(
+            base_url="http://localhost:8080/v1",
+            model="gemma-3-4b",
+            messages=_messages(),
+            schema=_Widget,
+            options={"temperature": 0.0, "think": False},
+        )
+    )
+
+    assert fake.calls[0][2] == {
+        "extra_body": {
+            "options": {"temperature": 0.0},
+            "chat_template_kwargs": {"enable_thinking": False},
+            "reasoning_effort": "none",
+        }
+    }
+
+
+def test_chat_structured_enable_thinking_skips_reasoning_effort(monkeypatch):
+    fake = _FakeAgent([_Widget(name="a", count=1)])
+    monkeypatch.setattr(chat_mod, "_build_agent", lambda **kw: fake)
+
+    _run_async(
+        chat_mod.chat_structured(
+            base_url="http://localhost:8080/v1",
+            model="gemma-3-4b",
+            messages=_messages(),
+            schema=_Widget,
+            options={"think": True},
+        )
+    )
+
+    extra_body = fake.calls[0][2]["extra_body"]
+    assert extra_body["chat_template_kwargs"] == {"enable_thinking": True}
+    assert "reasoning_effort" not in extra_body
+    assert "options" not in extra_body  # only "think" was in options
+
+
 def test_chat_structured_requires_last_message_user_role():
     with pytest.raises(ValueError, match="role='user'"):
         _run_async(

tests/test_ollama.py

@@ -47,6 +47,7 @@ from comfydv.ollama import (
     OllamaHeaderBearerToken,
     OllamaHeaderCustom,
     OllamaHistoryLength,
+    OllamaOptionDisableThinking,
     OllamaOptionExtraBody,
     OllamaOptionMaxTokens,
     OllamaOptionRepeatPenalty,
@@ -708,6 +709,29 @@ class TestUS5ComposableOptions:
         (opts,) = OllamaOptionRepeatPenalty().set_repeat_penalty(repeat_penalty=1.1)
         assert opts == {"repeat_penalty": 1.1}
 
+    def test_disable_thinking_default_sets_think_false(self):
+        """ADR-010: default True (disable thinking) merges think=False —
+        every LLMProvider.chat()/chat_structured() implementation pops this
+        key out of options and translates it to its own wire shape."""
+        (opts,) = OllamaOptionDisableThinking().set_disable_thinking(
+            disable_thinking=True
+        )
+        assert opts == {"think": False}
+
+    def test_disable_thinking_toggled_off_sets_think_true(self):
+        """Explicitly re-enabling thinking (e.g. to override a server-side
+        default) is the inverse: disable_thinking=False -> think=True."""
+        (opts,) = OllamaOptionDisableThinking().set_disable_thinking(
+            disable_thinking=False
+        )
+        assert opts == {"think": True}
+
+    def test_disable_thinking_merges_existing_options(self):
+        (opts,) = OllamaOptionDisableThinking().set_disable_thinking(
+            disable_thinking=True, options={"temperature": 0.5}
+        )
+        assert opts == {"temperature": 0.5, "think": False}
+
     def test_extra_body_merges_json(self):
         (opts,) = OllamaOptionExtraBody().set_extra_body(
             extra_body_json='{"stop": ["</s>"]}', options={"temperature": 0.0}

tests/test_ollama_provider.py

@@ -191,6 +191,72 @@ def test_chat_uses_api_chat_and_returns_content(monkeypatch):
     assert result == "hello there"
 
 
+def test_pop_think_splits_key_out_without_mutating_caller_dict():
+    """ADR-010: _pop_think must not mutate the caller's own options dict —
+    it's shared across retry attempts and possibly other calls."""
+    original = {"num_ctx": 4096, "think": False}
+    remaining, think = provider_mod._pop_think(original)
+
+    assert think is False
+    assert remaining == {"num_ctx": 4096}
+    assert original == {"num_ctx": 4096, "think": False}  # untouched
+
+
+def test_pop_think_absent_key_returns_none():
+    remaining, think = provider_mod._pop_think({"num_ctx": 4096})
+    assert think is None
+    assert remaining == {"num_ctx": 4096}
+
+
+def test_pop_think_only_key_present_returns_none_options():
+    remaining, think = provider_mod._pop_think({"think": True})
+    assert think is True
+    assert remaining is None
+
+
+def test_pop_think_none_input_returns_none_options_and_none_think():
+    assert provider_mod._pop_think(None) == (None, None)
+
+
+def test_chat_disable_thinking_sets_top_level_field_not_nested(monkeypatch):
+    """ADR-010: 'think' must be a top-level payload field, not nested inside
+    'options' — confirmed live that Ollama silently ignores it there."""
+    captured = {}
+
+    async def fake_post(url, payload, *, timeout=120.0, headers=None):
+        captured.update(payload)
+        return {"message": {"content": "pong"}}
+
+    monkeypatch.setattr(provider_mod, "_post_json", fake_post)
+    _run_async(
+        OllamaProvider("http://localhost:11434").chat(
+            "llama3",
+            [Message(role="user", content="hi")],
+            options={"num_ctx": 4096, "think": False},
+        )
+    )
+
+    assert captured["think"] is False
+    assert captured["options"] == {"num_ctx": 4096}  # "think" popped out
+
+
+def test_chat_no_think_key_omits_top_level_field(monkeypatch):
+    captured = {}
+
+    async def fake_post(url, payload, *, timeout=120.0, headers=None):
+        captured.update(payload)
+        return {"message": {"content": "pong"}}
+
+    monkeypatch.setattr(provider_mod, "_post_json", fake_post)
+    _run_async(
+        OllamaProvider("http://localhost:11434").chat(
+            "llama3", [Message(role="user", content="hi")]
+        )
+    )
+
+    assert "think" not in captured
+
+
 def test_chat_second_identical_call_is_cached(monkeypatch):
     calls = {"n": 0}
 
@@ -623,6 +689,32 @@ def test_chat_structured_caches_after_successful_validation(monkeypatch):
     assert calls["n"] == 1
 
 
+def test_chat_structured_disable_thinking_sets_top_level_field(monkeypatch):
+    from pydantic import BaseModel
+
+    class Widget(BaseModel):
+        name: str
+
+    captured = {}
+
+    async def fake_post(url, payload, *, timeout=120.0, headers=None):
+        captured.update(payload)
+        return {"message": {"content": '{"name": "x"}'}}
+
+    monkeypatch.setattr(provider_mod, "_post_json", fake_post)
+    _run_async(
+        OllamaProvider("http://localhost:11434").chat_structured(
+            "llama3",
+            [Message(role="user", content="hi")],
+            Widget,
+            options={"num_ctx": 4096, "think": False},
+        )
+    )
+
+    assert captured["think"] is False
+    assert captured["options"] == {"num_ctx": 4096}
+
+
 # ---------------------------------------------------------------------------
 # _run_async — regression coverage for a bug found running the real
 # docker-compose ComfyUI harness (not caught by any prior test, and not