From 019b76152c9d8022743ec590f0338af083f8f002 Mon Sep 17 00:00:00 2001 From: amtulifra Date: Sun, 23 Aug 2026 04:24:40 +0530 Subject: [PATCH 1/2] fix(llm): set reasoning_effort for known Ollama chain-of-thought models (#2932) The Gemini backend already sends reasoning_effort="low" via its static BACKENDS config, but Ollama never sent one at all. Local reasoning models served through Ollama (nemotron, deepseek-r1, qwq) narrate at length before answering without it, burning most of --api-timeout on reasoning tokens instead of the JSON reply and causing bisection failures/dropped files on real extraction chunks. Add a model-name-based resolver that sends reasoning_effort="high" for recognized reasoning models and omits it for everything else (unchanged default behaviour), with GRAPHIFY_OLLAMA_REASONING_EFFORT as an explicit override ("none"/"omit" disables it outright). Wired into both extract_files_direct's OpenAI-compat dispatch and _call_llm (the --dedup-llm tiebreaker path), which had the identical static cfg lookup and would hit the same failure mode. --- graphify/llm.py | 50 +++++++++++- tests/test_llm_backends.py | 153 +++++++++++++++++++++++++++++++++++++ 2 files changed, 200 insertions(+), 3 deletions(-) diff --git a/graphify/llm.py b/graphify/llm.py index 5e410af21..e6db59fa0 100644 --- a/graphify/llm.py +++ b/graphify/llm.py @@ -375,6 +375,43 @@ def _resolve_temperature(default: float | None, model: str = "") -> float | None return default +# Model-name fragments for Ollama-served chain-of-thought models. Ollama's +# OpenAI-compat endpoint honours `reasoning_effort` for these, and without it +# they narrate at length before answering, burning most of --api-timeout on +# reasoning tokens instead of the JSON reply (#2932). Matched case-insensitively +# against the resolved model tag (e.g. "deepseek-r1:32b"). +_OLLAMA_REASONING_MODEL_MARKERS = ("nemotron", "deepseek-r1", "qwq") + + +def _model_is_ollama_reasoning_model(model: str) -> bool: + """True if `model` is a known Ollama chain-of-thought model (#2932).""" + m = (model or "").lower() + return any(marker in m for marker in _OLLAMA_REASONING_MODEL_MARKERS) + + +def _resolve_ollama_reasoning_effort(model: str) -> str | None: + """Resolve the `reasoning_effort` to send for the ollama backend (#2932). + + Precedence: + 1. GRAPHIFY_OLLAMA_REASONING_EFFORT env var, if set: + - "none"/"omit" (case-insensitive) sends no reasoning_effort at all; + - any other value (e.g. "low", "medium", "high") is sent verbatim. + 2. Otherwise, known reasoning models (nemotron, deepseek-r1, qwq) get + "high" — without it they spend most of the request narrating instead + of answering, and time out on real extraction chunks. + 3. Otherwise None (omit the parameter — unchanged default behaviour for + non-reasoning models like qwen2.5-coder). + """ + raw = os.environ.get("GRAPHIFY_OLLAMA_REASONING_EFFORT", "").strip() + if raw: + if raw.lower() in ("none", "omit"): + return None + return raw + if _model_is_ollama_reasoning_model(model): + return "high" + return None + + def _bedrock_inference_config(max_tokens: int, model: str = "") -> dict: """Build Bedrock inferenceConfig, honouring GRAPHIFY_LLM_TEMPERATURE. @@ -1980,7 +2017,10 @@ def extract_files_direct( mdl, user_msg, temperature=_resolve_temperature(cfg.get("temperature", 0), mdl), - reasoning_effort=cfg.get("reasoning_effort"), + reasoning_effort=( + _resolve_ollama_reasoning_effort(mdl) if backend == "ollama" + else cfg.get("reasoning_effort") + ), # Honour max_completion_tokens (gemini) or the older max_tokens key # (ollama/deepseek/kimi/openai) -- most openai-compat configs define the # latter, so reading only max_completion_tokens silently capped their @@ -2954,8 +2994,12 @@ def _rec(inp, out) -> None: temperature = _resolve_temperature(cfg.get("temperature", 0), mdl) if temperature is not None: kwargs["temperature"] = temperature - if cfg.get("reasoning_effort"): - kwargs["reasoning_effort"] = cfg["reasoning_effort"] + reasoning_effort = ( + _resolve_ollama_reasoning_effort(mdl) if backend == "ollama" + else cfg.get("reasoning_effort") + ) + if reasoning_effort: + kwargs["reasoning_effort"] = reasoning_effort # Custom providers can override via providers.json `extra_body`; falls back # to the moonshot default to preserve existing behavior. if cfg.get("extra_body") is not None: diff --git a/tests/test_llm_backends.py b/tests/test_llm_backends.py index 4480eff62..5c0ba7446 100644 --- a/tests/test_llm_backends.py +++ b/tests/test_llm_backends.py @@ -1088,6 +1088,106 @@ def test_openai_compat_env_var_temperature_applied(tmp_path, monkeypatch): assert captured.get("temperature") == 0.3 +# --------------------------------------------------------------------------- +# Ollama reasoning_effort resolution (#2932): known chain-of-thought models +# get reasoning_effort="high" so they stop narrating past --api-timeout, and +# GRAPHIFY_OLLAMA_REASONING_EFFORT overrides. +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "model", + ["nemotron-mini", "deepseek-r1:32b", "qwq:32b", "QwQ", "NEMOTRON", "deepseek-r1"], +) +def test_model_is_ollama_reasoning_model_true_for_known_models(model): + assert llm._model_is_ollama_reasoning_model(model) is True + + +@pytest.mark.parametrize( + "model", + ["qwen2.5-coder:7b", "llama3.1", "gpt-4.1-mini", "", "deepseek-v4-flash"], +) +def test_model_is_ollama_reasoning_model_false_for_normal_models(model): + assert llm._model_is_ollama_reasoning_model(model) is False + + +def test_resolve_ollama_reasoning_effort_high_for_reasoning_model(monkeypatch): + monkeypatch.delenv("GRAPHIFY_OLLAMA_REASONING_EFFORT", raising=False) + assert llm._resolve_ollama_reasoning_effort("deepseek-r1:32b") == "high" + + +def test_resolve_ollama_reasoning_effort_none_for_normal_model(monkeypatch): + monkeypatch.delenv("GRAPHIFY_OLLAMA_REASONING_EFFORT", raising=False) + assert llm._resolve_ollama_reasoning_effort("qwen2.5-coder:7b") is None + + +def test_resolve_ollama_reasoning_effort_env_var_overrides(monkeypatch): + monkeypatch.setenv("GRAPHIFY_OLLAMA_REASONING_EFFORT", "medium") + # env var wins even for a normal model (explicit user choice) + assert llm._resolve_ollama_reasoning_effort("qwen2.5-coder:7b") == "medium" + assert llm._resolve_ollama_reasoning_effort("deepseek-r1:32b") == "medium" + + +def test_resolve_ollama_reasoning_effort_env_var_none_omits(monkeypatch): + monkeypatch.setenv("GRAPHIFY_OLLAMA_REASONING_EFFORT", "none") + # env var can force it off even for a known reasoning model + assert llm._resolve_ollama_reasoning_effort("deepseek-r1:32b") is None + + +def test_openai_compat_sends_reasoning_effort_for_ollama_reasoning_model(tmp_path, monkeypatch): + # Regression for #2932: without reasoning_effort, Ollama reasoning models + # (nemotron, deepseek-r1, qwq) burn most of --api-timeout on narration. + _clear_backend_env(monkeypatch) + monkeypatch.delenv("GRAPHIFY_OLLAMA_REASONING_EFFORT", raising=False) + captured = _install_capturing_openai(monkeypatch) + (tmp_path / "f.py").write_text("x = 1\n") + + llm.extract_files_direct([tmp_path / "f.py"], backend="ollama", + model="deepseek-r1:32b", root=tmp_path) + + assert captured.get("reasoning_effort") == "high" + + +def test_openai_compat_omits_reasoning_effort_for_ollama_normal_model(tmp_path, monkeypatch): + _clear_backend_env(monkeypatch) + monkeypatch.delenv("GRAPHIFY_OLLAMA_REASONING_EFFORT", raising=False) + captured = _install_capturing_openai(monkeypatch) + (tmp_path / "f.py").write_text("x = 1\n") + + llm.extract_files_direct([tmp_path / "f.py"], backend="ollama", + model="qwen2.5-coder:7b", root=tmp_path) + + assert "reasoning_effort" not in captured, ( + "non-reasoning ollama models must not get a reasoning_effort key at all" + ) + + +def test_openai_compat_env_var_reasoning_effort_applied_to_ollama(tmp_path, monkeypatch): + _clear_backend_env(monkeypatch) + monkeypatch.setenv("GRAPHIFY_OLLAMA_REASONING_EFFORT", "low") + captured = _install_capturing_openai(monkeypatch) + (tmp_path / "f.py").write_text("x = 1\n") + + llm.extract_files_direct([tmp_path / "f.py"], backend="ollama", + model="qwen2.5-coder:7b", root=tmp_path) + + assert captured.get("reasoning_effort") == "low" + + +def test_gemini_reasoning_effort_unaffected_by_ollama_env_var(tmp_path, monkeypatch): + # The dynamic resolver is gated on backend == "ollama"; gemini keeps using + # its static BACKENDS config regardless of the ollama-only env var. + _clear_backend_env(monkeypatch) + monkeypatch.setenv("GOOGLE_API_KEY", "google-key") + monkeypatch.setenv("GRAPHIFY_OLLAMA_REASONING_EFFORT", "medium") + captured = _install_capturing_openai(monkeypatch) + (tmp_path / "f.py").write_text("x = 1\n") + + llm.extract_files_direct([tmp_path / "f.py"], backend="gemini", root=tmp_path) + + assert captured.get("reasoning_effort") == "low" + + def test_native_extraction_prompt_requests_hyperedges(): """The native-backend prompt must request hyperedges, like the skill's extraction-spec does — otherwise `graphify extract --backend X` silently @@ -1332,6 +1432,59 @@ def create(self, **_): assert ctor_kwargs.get("max_retries", 0) >= 5, ctor_kwargs +def test_call_llm_ollama_reasoning_model_gets_reasoning_effort(monkeypatch): + """#2932: _call_llm (the --dedup-llm tiebreaker path) mirrors + extract_files_direct's backend dispatch (per its own docstring), so it had + the identical static cfg.get("reasoning_effort") gap for ollama.""" + import sys + import types + + call_kwargs = {} + + class _FakeOpenAI: + def __init__(self, *_, **__): + self.chat = self + self.completions = self + + def create(self, **kwargs): + call_kwargs.update(kwargs) + return _fake_openai_response("ok", finish_reason="stop", completion_tokens=1) + + fake_module = types.ModuleType("openai") + fake_module.OpenAI = _FakeOpenAI + monkeypatch.setitem(sys.modules, "openai", fake_module) + monkeypatch.setattr(llm, "_get_backend_api_key", lambda _b: "ollama") + monkeypatch.delenv("GRAPHIFY_OLLAMA_REASONING_EFFORT", raising=False) + + llm._call_llm("hi", backend="ollama", model="deepseek-r1:32b") + assert call_kwargs.get("reasoning_effort") == "high" + + +def test_call_llm_ollama_normal_model_gets_no_reasoning_effort(monkeypatch): + import sys + import types + + call_kwargs = {} + + class _FakeOpenAI: + def __init__(self, *_, **__): + self.chat = self + self.completions = self + + def create(self, **kwargs): + call_kwargs.update(kwargs) + return _fake_openai_response("ok", finish_reason="stop", completion_tokens=1) + + fake_module = types.ModuleType("openai") + fake_module.OpenAI = _FakeOpenAI + monkeypatch.setitem(sys.modules, "openai", fake_module) + monkeypatch.setattr(llm, "_get_backend_api_key", lambda _b: "ollama") + monkeypatch.delenv("GRAPHIFY_OLLAMA_REASONING_EFFORT", raising=False) + + llm._call_llm("hi", backend="ollama", model="qwen2.5-coder:7b") + assert "reasoning_effort" not in call_kwargs + + def test_adaptive_retry_does_not_bisect_a_hollow_response(tmp_path, monkeypatch): """#2880: a hollow response is retried as-is, never bisected. From fe15076832b976429ac8a3ed4100846baaad6550 Mon Sep 17 00:00:00 2001 From: amtulifra Date: Sun, 23 Aug 2026 04:57:13 +0530 Subject: [PATCH 2/2] fix(llm): compose ollama reasoning_effort resolver with static config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review flagged that the ollama branch unconditionally used the dynamic resolver, discarding cfg.get("reasoning_effort") outright instead of falling back to it. Not reachable today — BACKENDS["ollama"] carries no such key, and a same-named custom provider can't add one (providers.json entries matching an existing BACKENDS name are skipped) — but the resolver silently dropping a static default if that ever changes is a real footgun. Pass cfg.get("reasoning_effort") through as the resolver's bottom-of-precedence default instead of bypassing it, at both call sites. --- graphify/llm.py | 20 +++++++++++-------- tests/test_llm_backends.py | 40 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/graphify/llm.py b/graphify/llm.py index e6db59fa0..e38b5a498 100644 --- a/graphify/llm.py +++ b/graphify/llm.py @@ -389,7 +389,7 @@ def _model_is_ollama_reasoning_model(model: str) -> bool: return any(marker in m for marker in _OLLAMA_REASONING_MODEL_MARKERS) -def _resolve_ollama_reasoning_effort(model: str) -> str | None: +def _resolve_ollama_reasoning_effort(model: str, default: str | None = None) -> str | None: """Resolve the `reasoning_effort` to send for the ollama backend (#2932). Precedence: @@ -399,8 +399,12 @@ def _resolve_ollama_reasoning_effort(model: str) -> str | None: 2. Otherwise, known reasoning models (nemotron, deepseek-r1, qwq) get "high" — without it they spend most of the request narrating instead of answering, and time out on real extraction chunks. - 3. Otherwise None (omit the parameter — unchanged default behaviour for - non-reasoning models like qwen2.5-coder). + 3. Otherwise `default` — the backend's static BACKENDS[...] config value, + if the caller passes one. BACKENDS["ollama"] carries no such key today + (and a same-named custom provider in providers.json cannot add one: + _load_custom_providers skips any name already in BACKENDS), so this is + always None in practice right now. It exists so a future static + default is composed with, not silently discarded by, this resolver. """ raw = os.environ.get("GRAPHIFY_OLLAMA_REASONING_EFFORT", "").strip() if raw: @@ -409,7 +413,7 @@ def _resolve_ollama_reasoning_effort(model: str) -> str | None: return raw if _model_is_ollama_reasoning_model(model): return "high" - return None + return default def _bedrock_inference_config(max_tokens: int, model: str = "") -> dict: @@ -2018,8 +2022,8 @@ def extract_files_direct( user_msg, temperature=_resolve_temperature(cfg.get("temperature", 0), mdl), reasoning_effort=( - _resolve_ollama_reasoning_effort(mdl) if backend == "ollama" - else cfg.get("reasoning_effort") + _resolve_ollama_reasoning_effort(mdl, cfg.get("reasoning_effort")) + if backend == "ollama" else cfg.get("reasoning_effort") ), # Honour max_completion_tokens (gemini) or the older max_tokens key # (ollama/deepseek/kimi/openai) -- most openai-compat configs define the @@ -2995,8 +2999,8 @@ def _rec(inp, out) -> None: if temperature is not None: kwargs["temperature"] = temperature reasoning_effort = ( - _resolve_ollama_reasoning_effort(mdl) if backend == "ollama" - else cfg.get("reasoning_effort") + _resolve_ollama_reasoning_effort(mdl, cfg.get("reasoning_effort")) + if backend == "ollama" else cfg.get("reasoning_effort") ) if reasoning_effort: kwargs["reasoning_effort"] = reasoning_effort diff --git a/tests/test_llm_backends.py b/tests/test_llm_backends.py index 5c0ba7446..50e41e075 100644 --- a/tests/test_llm_backends.py +++ b/tests/test_llm_backends.py @@ -1134,6 +1134,29 @@ def test_resolve_ollama_reasoning_effort_env_var_none_omits(monkeypatch): assert llm._resolve_ollama_reasoning_effort("deepseek-r1:32b") is None +def test_resolve_ollama_reasoning_effort_falls_back_to_static_default(monkeypatch): + # A static BACKENDS[...]["reasoning_effort"] (mirroring how gemini's config + # works today) must not be silently discarded for a non-reasoning model + # with no env override — it's the bottom of the precedence chain, not + # dropped. Reachability: BACKENDS["ollama"] itself carries no such key, and + # a same-named custom provider can't add one (providers named in BACKENDS + # are skipped by _load_custom_providers), so `default` is always None in + # today's real usage — this guards the resolver's contract regardless. + monkeypatch.delenv("GRAPHIFY_OLLAMA_REASONING_EFFORT", raising=False) + assert llm._resolve_ollama_reasoning_effort("qwen2.5-coder:7b", default="medium") == "medium" + + +def test_resolve_ollama_reasoning_effort_model_match_wins_over_static_default(monkeypatch): + monkeypatch.delenv("GRAPHIFY_OLLAMA_REASONING_EFFORT", raising=False) + # a known reasoning model still gets "high", not the unrelated static default + assert llm._resolve_ollama_reasoning_effort("deepseek-r1:32b", default="low") == "high" + + +def test_resolve_ollama_reasoning_effort_env_var_wins_over_static_default(monkeypatch): + monkeypatch.setenv("GRAPHIFY_OLLAMA_REASONING_EFFORT", "medium") + assert llm._resolve_ollama_reasoning_effort("qwen2.5-coder:7b", default="low") == "medium" + + def test_openai_compat_sends_reasoning_effort_for_ollama_reasoning_model(tmp_path, monkeypatch): # Regression for #2932: without reasoning_effort, Ollama reasoning models # (nemotron, deepseek-r1, qwq) burn most of --api-timeout on narration. @@ -1162,6 +1185,23 @@ def test_openai_compat_omits_reasoning_effort_for_ollama_normal_model(tmp_path, ) +def test_extract_files_direct_honours_ollama_static_config_default(tmp_path, monkeypatch): + """A static BACKENDS["ollama"]["reasoning_effort"] must reach the actual + request as the bottom-of-precedence fallback, not be silently discarded + just because backend == "ollama" — the resolver must compose with the + static config, not replace it outright.""" + _clear_backend_env(monkeypatch) + monkeypatch.delenv("GRAPHIFY_OLLAMA_REASONING_EFFORT", raising=False) + monkeypatch.setitem(llm.BACKENDS["ollama"], "reasoning_effort", "medium") + captured = _install_capturing_openai(monkeypatch) + (tmp_path / "f.py").write_text("x = 1\n") + + llm.extract_files_direct([tmp_path / "f.py"], backend="ollama", + model="qwen2.5-coder:7b", root=tmp_path) + + assert captured.get("reasoning_effort") == "medium" + + def test_openai_compat_env_var_reasoning_effort_applied_to_ollama(tmp_path, monkeypatch): _clear_backend_env(monkeypatch) monkeypatch.setenv("GRAPHIFY_OLLAMA_REASONING_EFFORT", "low")