diff --git a/graphify/llm.py b/graphify/llm.py index 5e410af21..e38b5a498 100644 --- a/graphify/llm.py +++ b/graphify/llm.py @@ -375,6 +375,47 @@ 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, default: str | None = None) -> 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 `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: + if raw.lower() in ("none", "omit"): + return None + return raw + if _model_is_ollama_reasoning_model(model): + return "high" + return default + + def _bedrock_inference_config(max_tokens: int, model: str = "") -> dict: """Build Bedrock inferenceConfig, honouring GRAPHIFY_LLM_TEMPERATURE. @@ -1980,7 +2021,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, 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 # latter, so reading only max_completion_tokens silently capped their @@ -2954,8 +2998,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, cfg.get("reasoning_effort")) + 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..50e41e075 100644 --- a/tests/test_llm_backends.py +++ b/tests/test_llm_backends.py @@ -1088,6 +1088,146 @@ 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_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. + _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_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") + 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 +1472,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.