From 2c7419723bfcfc6ddfa30e83882e9f65e7ae2829 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Boschi?= Date: Mon, 10 Aug 2026 15:04:41 +0200 Subject: [PATCH] fix(ollama): make native think configurable via extra_body The native structured-output path hardcoded think=False, so gpt-oss models failed fact extraction (they require a thinking level). Rather than a model-name heuristic, merge the configured extra_body into the native /api/chat payload: top-level native fields (think, keep_alive, ...) pass through, and an "options" sub-dict merges into Ollama's generation options. Set HINDSIGHT_API_LLM_EXTRA_BODY='{"think": "low"}' for gpt-oss. Fixes #3246 --- .../engine/providers/openai_compatible_llm.py | 14 ++- .../tests/test_ollama_native_think.py | 111 ++++++++++++++++++ .../docs/developer/configuration.md | 7 +- .../references/developer/configuration.md | 7 +- 4 files changed, 136 insertions(+), 3 deletions(-) create mode 100644 hindsight-api-slim/tests/test_ollama_native_think.py diff --git a/hindsight-api-slim/hindsight_api/engine/providers/openai_compatible_llm.py b/hindsight-api-slim/hindsight_api/engine/providers/openai_compatible_llm.py index 051ae538eb..3edaab9882 100644 --- a/hindsight-api-slim/hindsight_api/engine/providers/openai_compatible_llm.py +++ b/hindsight-api-slim/hindsight_api/engine/providers/openai_compatible_llm.py @@ -1463,7 +1463,9 @@ async def _call_ollama_native( "model": self.model, "messages": messages, "stream": False, - "think": False, # Disable thinking for reasoning models (qwen3.5, etc.) + # Disable thinking by default (qwen3.5, etc.). Override via + # extra_body, e.g. {"think": "low"} for gpt-oss models (see #3246). + "think": False, } # Add schema as format parameter for structured output @@ -1480,6 +1482,16 @@ async def _call_ollama_native( options["num_predict"] = max_completion_tokens if temperature is not None: options["temperature"] = temperature + + # Merge configured extra_body into the native payload. Ollama's native + # /api/chat body has two tiers, unlike the OpenAI-compatible endpoint + # where the SDK flattens everything to top-level: native top-level + # fields (think, keep_alive, ...) pass through directly, while an + # "options" sub-dict merges into Ollama's generation options + # (seed, top_p, num_ctx, ...). User values win over the defaults above. + extra_body = dict(self._config_extra_body) + options.update(extra_body.pop("options", {})) + payload.update(extra_body) payload["options"] = options last_exception = None diff --git a/hindsight-api-slim/tests/test_ollama_native_think.py b/hindsight-api-slim/tests/test_ollama_native_think.py new file mode 100644 index 0000000000..e6b507fd92 --- /dev/null +++ b/hindsight-api-slim/tests/test_ollama_native_think.py @@ -0,0 +1,111 @@ +""" +Regression tests for Ollama native API extra_body handling. + +The native /api/chat payload has two tiers: native top-level fields (``think``, +``keep_alive``, ...) and a nested ``options`` object (``seed``, ``top_p``, +``num_ctx``, ...). Configured ``extra_body`` must reach both, so operators can +enable thinking for gpt-oss models (``{"think": "low"}``) or tune generation +options without a code change (see #3246). +""" + +import json +from unittest.mock import AsyncMock, patch + +import httpx +import pytest +from pydantic import BaseModel + +from hindsight_api.engine.providers.openai_compatible_llm import OpenAICompatibleLLM + + +class _SampleOutput(BaseModel): + summary: str + + +def _make_ollama_llm(model: str, extra_body: dict | None = None) -> OpenAICompatibleLLM: + return OpenAICompatibleLLM( + provider="ollama", + api_key="", + base_url="http://localhost:11434/v1", + model=model, + extra_body=extra_body, + ) + + +def _mock_ollama_response(content: dict) -> httpx.Response: + body = { + "model": "test-model", + "message": { + "role": "assistant", + "content": json.dumps(content), + }, + "done": True, + } + request = httpx.Request("POST", "http://localhost:11434/api/chat") + return httpx.Response(200, json=body, request=request) + + +async def _capture_payload(llm: OpenAICompatibleLLM) -> dict: + mock_client = AsyncMock() + mock_client.post.return_value = _mock_ollama_response({"summary": "test"}) + mock_client.__aenter__.return_value = mock_client + + with patch( + "hindsight_api.engine.providers.openai_compatible_llm.httpx.AsyncClient", + return_value=mock_client, + ): + await llm._call_ollama_native( + messages=[{"role": "user", "content": "hello"}], + response_format=_SampleOutput, + max_completion_tokens=512, + temperature=0.1, + max_retries=0, + initial_backoff=1.0, + max_backoff=10.0, + skip_validation=True, + ) + + request = mock_client.post.call_args + assert request is not None + return request.kwargs["json"] + + +@pytest.mark.asyncio +async def test_ollama_native_think_defaults_false(): + """Thinking is disabled by default and structured-output format is included.""" + payload = await _capture_payload(_make_ollama_llm("qwen3.5:2b")) + + assert payload["think"] is False + assert "format" in payload + assert payload["options"]["num_predict"] == 512 + assert payload["options"]["temperature"] == 0.1 + + +@pytest.mark.asyncio +async def test_ollama_native_think_override_via_extra_body(): + """extra_body top-level field overrides the think default (gpt-oss path).""" + payload = await _capture_payload(_make_ollama_llm("gpt-oss:20b", extra_body={"think": "low"})) + + assert payload["think"] == "low" + # Computed options are preserved alongside the top-level override. + assert payload["options"]["num_predict"] == 512 + assert payload["options"]["temperature"] == 0.1 + + +@pytest.mark.asyncio +async def test_ollama_native_options_merge_via_extra_body(): + """An extra_body "options" sub-dict merges into native generation options.""" + payload = await _capture_payload( + _make_ollama_llm( + "qwen3.5:2b", + extra_body={"options": {"seed": 42, "temperature": 0.9}}, + ) + ) + + # New option added, and a user value wins over the computed default. + assert payload["options"]["seed"] == 42 + assert payload["options"]["temperature"] == 0.9 + assert payload["options"]["num_predict"] == 512 + # "options" is not leaked as a top-level payload field. + assert "options" in payload + assert payload["think"] is False diff --git a/hindsight-docs/docs/developer/configuration.md b/hindsight-docs/docs/developer/configuration.md index cf9115656b..71c135fe1d 100644 --- a/hindsight-docs/docs/developer/configuration.md +++ b/hindsight-docs/docs/developer/configuration.md @@ -190,7 +190,7 @@ For non-English banks (especially CJK) and the language/extraction-language trad | `HINDSIGHT_API_LLM_OPENAI_SERVICE_TIER` | OpenAI service tier: `flex` for 50% cost savings (OpenAI Flex Processing) | None (default) | | `HINDSIGHT_API_LLM_BEDROCK_SERVICE_TIER` | Bedrock service tier: `flex` for 50% cost savings (best-effort inference), `priority` (guaranteed throughput), or `reserved` (provisioned capacity) | Unset (default tier) | | `HINDSIGHT_API_LLM_GEMINI_SERVICE_TIER` | Gemini service tier: `flex` for 50% cost savings (best-effort inference) | Unset (default tier) | -| `HINDSIGHT_API_LLM_EXTRA_BODY` | JSON dict of extra request-body params (e.g. `temperature`, `top_p`, `max_tokens`) merged into every LLM call. Applied across the OpenAI-compatible, Fireworks, Anthropic, Gemini/VertexAI and LiteLLM (incl. Bedrock/Router) providers. Each provider merges them in its own native parameter space, so use that provider's field names (e.g. `max_tokens` for OpenAI/Anthropic vs `max_output_tokens` for Gemini). Also useful for custom model servers (e.g. vLLM `chat_template_kwargs`). | `null` | +| `HINDSIGHT_API_LLM_EXTRA_BODY` | JSON dict of extra request-body params (e.g. `temperature`, `top_p`, `max_tokens`) merged into every LLM call. Applied across the OpenAI-compatible, Fireworks, Anthropic, Gemini/VertexAI and LiteLLM (incl. Bedrock/Router) providers. Each provider merges them in its own native parameter space, so use that provider's field names (e.g. `max_tokens` for OpenAI/Anthropic vs `max_output_tokens` for Gemini). The native Ollama structured-output path is a special case — see the note below. Also useful for custom model servers (e.g. vLLM `chat_template_kwargs`). | `null` | | `HINDSIGHT_API_LLM_DEFAULT_HEADERS` | JSON dict passed as `default_headers` to provider SDK clients. Used by operators routing through proxies / request-tracing middleware (e.g. Cloudflare AI Gateway, Helicone, corporate proxies). Currently wired into the Anthropic provider; other providers can opt in. | `null` | | `HINDSIGHT_API_LLM_STRICT_SCHEMA` | Grammar-enforce structured output via `json_schema` `strict: true` instead of the soft "schema-in-prompt + `json_object`" path. Typed Pydantic response models are serialized directly into the OpenAI strict subset: every object rejects additional properties, every declared property is required, and nullable fields remain nullable. Use it with weaker self-hosted models that return prose preambles, markdown ` ```json ` fences, or invalid JSON — which otherwise fail to parse and wedge retain/consolidation. Applies to OpenAI-compatible backends (OpenAI, llama.cpp, vLLM), Codex, and LiteLLM; Gemini already enforces its native `response_schema` regardless, and providers without a strict mode ignore it. | `false` | | `HINDSIGHT_API_LLM_STRICT_SCHEMA_RETAIN` | Override `HINDSIGHT_API_LLM_STRICT_SCHEMA` for retain (fact extraction) only. Applies to both the streaming and batch extraction paths. | Inherits global | @@ -205,6 +205,11 @@ For non-English banks (especially CJK) and the language/extraction-language trad When `HINDSIGHT_API_LLM_PROVIDER=ollama`, Hindsight no longer sends the previous native API default `num_ctx=16384` unless you set it explicitly. To keep the old request behavior, set `HINDSIGHT_API_LLM_OLLAMA_NUM_CTX=16384`; otherwise Ollama uses the model Modelfile or server default. +**`HINDSIGHT_API_LLM_EXTRA_BODY` on the native Ollama path.** For structured-output calls, Ollama uses its native `/api/chat` API, whose request body has *two tiers* — this differs from the OpenAI-compatible endpoint, where the SDK flattens everything to top-level. On the native path `extra_body` is split accordingly: + +- **Top-level native fields** (`think`, `keep_alive`, ...) pass through directly. For example, gpt-oss models require a thinking level for structured extraction, so set `HINDSIGHT_API_LLM_EXTRA_BODY='{"think": "low"}'` (thinking is disabled by default). +- **Generation parameters** (`seed`, `top_p`, `top_k`, `num_ctx`, `temperature`, ...) live under Ollama's `options` object, so nest them: `HINDSIGHT_API_LLM_EXTRA_BODY='{"options": {"seed": 42, "top_p": 0.9}}'`. On the OpenAI-compatible endpoints these same params are top-level instead. + **Provider Examples** ```bash diff --git a/skills/hindsight-docs/references/developer/configuration.md b/skills/hindsight-docs/references/developer/configuration.md index af4c41adfa..27bf10354f 100644 --- a/skills/hindsight-docs/references/developer/configuration.md +++ b/skills/hindsight-docs/references/developer/configuration.md @@ -190,7 +190,7 @@ For non-English banks (especially CJK) and the language/extraction-language trad | `HINDSIGHT_API_LLM_OPENAI_SERVICE_TIER` | OpenAI service tier: `flex` for 50% cost savings (OpenAI Flex Processing) | None (default) | | `HINDSIGHT_API_LLM_BEDROCK_SERVICE_TIER` | Bedrock service tier: `flex` for 50% cost savings (best-effort inference), `priority` (guaranteed throughput), or `reserved` (provisioned capacity) | Unset (default tier) | | `HINDSIGHT_API_LLM_GEMINI_SERVICE_TIER` | Gemini service tier: `flex` for 50% cost savings (best-effort inference) | Unset (default tier) | -| `HINDSIGHT_API_LLM_EXTRA_BODY` | JSON dict of extra request-body params (e.g. `temperature`, `top_p`, `max_tokens`) merged into every LLM call. Applied across the OpenAI-compatible, Fireworks, Anthropic, Gemini/VertexAI and LiteLLM (incl. Bedrock/Router) providers. Each provider merges them in its own native parameter space, so use that provider's field names (e.g. `max_tokens` for OpenAI/Anthropic vs `max_output_tokens` for Gemini). Also useful for custom model servers (e.g. vLLM `chat_template_kwargs`). | `null` | +| `HINDSIGHT_API_LLM_EXTRA_BODY` | JSON dict of extra request-body params (e.g. `temperature`, `top_p`, `max_tokens`) merged into every LLM call. Applied across the OpenAI-compatible, Fireworks, Anthropic, Gemini/VertexAI and LiteLLM (incl. Bedrock/Router) providers. Each provider merges them in its own native parameter space, so use that provider's field names (e.g. `max_tokens` for OpenAI/Anthropic vs `max_output_tokens` for Gemini). The native Ollama structured-output path is a special case — see the note below. Also useful for custom model servers (e.g. vLLM `chat_template_kwargs`). | `null` | | `HINDSIGHT_API_LLM_DEFAULT_HEADERS` | JSON dict passed as `default_headers` to provider SDK clients. Used by operators routing through proxies / request-tracing middleware (e.g. Cloudflare AI Gateway, Helicone, corporate proxies). Currently wired into the Anthropic provider; other providers can opt in. | `null` | | `HINDSIGHT_API_LLM_STRICT_SCHEMA` | Grammar-enforce structured output via `json_schema` `strict: true` instead of the soft "schema-in-prompt + `json_object`" path. Typed Pydantic response models are serialized directly into the OpenAI strict subset: every object rejects additional properties, every declared property is required, and nullable fields remain nullable. Use it with weaker self-hosted models that return prose preambles, markdown ` ```json ` fences, or invalid JSON — which otherwise fail to parse and wedge retain/consolidation. Applies to OpenAI-compatible backends (OpenAI, llama.cpp, vLLM), Codex, and LiteLLM; Gemini already enforces its native `response_schema` regardless, and providers without a strict mode ignore it. | `false` | | `HINDSIGHT_API_LLM_STRICT_SCHEMA_RETAIN` | Override `HINDSIGHT_API_LLM_STRICT_SCHEMA` for retain (fact extraction) only. Applies to both the streaming and batch extraction paths. | Inherits global | @@ -205,6 +205,11 @@ For non-English banks (especially CJK) and the language/extraction-language trad When `HINDSIGHT_API_LLM_PROVIDER=ollama`, Hindsight no longer sends the previous native API default `num_ctx=16384` unless you set it explicitly. To keep the old request behavior, set `HINDSIGHT_API_LLM_OLLAMA_NUM_CTX=16384`; otherwise Ollama uses the model Modelfile or server default. +**`HINDSIGHT_API_LLM_EXTRA_BODY` on the native Ollama path.** For structured-output calls, Ollama uses its native `/api/chat` API, whose request body has *two tiers* — this differs from the OpenAI-compatible endpoint, where the SDK flattens everything to top-level. On the native path `extra_body` is split accordingly: + +- **Top-level native fields** (`think`, `keep_alive`, ...) pass through directly. For example, gpt-oss models require a thinking level for structured extraction, so set `HINDSIGHT_API_LLM_EXTRA_BODY='{"think": "low"}'` (thinking is disabled by default). +- **Generation parameters** (`seed`, `top_p`, `top_k`, `num_ctx`, `temperature`, ...) live under Ollama's `options` object, so nest them: `HINDSIGHT_API_LLM_EXTRA_BODY='{"options": {"seed": 42, "top_p": 0.9}}'`. On the OpenAI-compatible endpoints these same params are top-level instead. + **Provider Examples** ```bash