fix: send reasoning_effort for reasoning-tuned local models via ollama backend - #2933
fix: send reasoning_effort for reasoning-tuned local models via ollama backend#2933AutogenousEngine wants to merge 1 commit into
Conversation
…a backend The ollama backend never set reasoning_effort at all -- only the gemini backend config did. Confirmed empirically against nemotron-3-super:120b via raw /v1/chat/completions calls: Ollama's OpenAI-compat endpoint does honor reasoning_effort, and "high" roughly halves both reasoning length and total completion tokens versus omitting it. Since the narration shares the same request's wall-clock and the client's --api-timeout, an unset reasoning_effort on a verbose reasoning model can burn most of that budget on narration alone, pushing a real multi-file extraction chunk into repeated bisect-and-retry and still failing at a single file with nothing left to bisect (Graphify-Labs#2932). Adds _resolve_reasoning_effort(cfg, model, backend): a GRAPHIFY_OLLAMA_REASONING_EFFORT env override (accepting "none"/"omit"/"default" to force omission, mirroring GRAPHIFY_LLM_TEMPERATURE's convention), then the backend config's own default (gemini, unchanged), then "high" for ollama models matching the same reasoning families _THINK_BLOCK_RE already recognizes (nemotron, deepseek-r1, qwq), then None otherwise -- so non-reasoning models served through ollama (the backend's own default is qwen2.5-coder:7b) are unaffected. Fixes Graphify-Labs#2932
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Adds _resolve_reasoning_effort (plus _ollama_model_is_reasoning) to default reasoning_effort="high" for recognised local reasoning families (nemotron, deepseek-r1, qwq) on the Ollama backend, with a GRAPHIFY_OLLAMA_REASONING_EFFORT env override and config-default precedence. Wires it into extract_files_direct and _call_llm in place of the raw cfg.get("reasoning_effort") reads, and covers the precedence rules in a new tests/test_ollama_reasoning_effort.py.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 756 functions depend on the 183 functions this change touches.
Health — this change adds coupling hotspots:
- new:
deduplicate_entities()— 63 callers, 21 callees - new:
build_merge()— 46 callers, 14 callees - new:
extract_files_direct()— 17 callers, 21 callees - new:
extract_corpus_parallel()— 26 callers, 11 callees - new:
_call_claude_cli()— 31 callers, 9 callees - new:
dispatch_command()— 2 callers, 119 callees - new:
_extract_with_adaptive_retry()— 22 callers, 10 callees - new:
_call_llm()— 11 callers, 19 callees - …and 15 more — each is listed as a finding
Verification — 756 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 480 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify \_call\_llm.
The verifier did not have enough to check \_call\_llm, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: not verifiable: all 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)
Could not verify: Could not verify extract\_files\_direct.
The verifier did not have enough to check extract\_files\_direct, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `root` is annotated `Path` — outside the synthesizable primitive/collection set
· 23 more finding(s) on lines outside this diff (see the check run).
Fixes #2932.
Problem
The
ollamabackend config never setreasoning_effortat all -- onlygeminidid ("reasoning_effort": "low"). Benchmarkingnemotron-3-super:120bagainst Claude/GLM-5.2 on identical corpora, it produced consistently sparser extraction, and on a larger corpus dropped a file entirely after exhausting the bisect-and-retry timeout path. No JSON parsing was involved (.contentwas never hollow) -- see #2932 for the full empirical writeup, including raw/v1/chat/completionsmeasurements showingreasoning_effort: "high"roughly halves both reasoning length and completion tokens for this model relative to omitting it or"low".Since the model's chain-of-thought narration (landing in a separate
message.reasoningfield on Ollama's OpenAI-compat endpoint) shares the same request's wall-clock and the client's--api-timeout, an unsetreasoning_efforton a verbose local reasoning model can burn most of that budget on narration alone -- which is what pushed the real multi-file chunk into repeated bisection and eventual file loss.Fix
Adds
_resolve_reasoning_effort(cfg, model, backend), used at both call sites that previously readcfg.get("reasoning_effort")directly. Precedence:GRAPHIFY_OLLAMA_REASONING_EFFORTenv var (ollama only) -- explicit override; accepts"none"/"omit"/"default"to force omission even for a recognized reasoning model, mirroringGRAPHIFY_LLM_TEMPERATURE's existing convention.gemini, unchanged)."high"forollamamodels matching the same reasoning-model families_THINK_BLOCK_RE's comment already names (nemotron,deepseek-r1,qwq) -- reusing recognition the codebase already committed to elsewhere, not inventing a new category.Noneotherwise -- every other backend/model is unaffected, including non-reasoning models served through ollama (the backend's own default model isqwen2.5-coder:7b).Testing
tests/test_ollama_reasoning_effort.py(8 tests): family matching (including a provider-prefixednvidia/nemotron-3-superform, mirroring_model_requires_default_temperature's existing prefix-stripping), the default-for-reasoning-models / none-for-others split, non-ollama backends being unaffected by the env var, env override winning over the default (including for non-reasoning models -- an explicit choice always wins), thenone/omit/defaultforce-omit sentinels, and a backend-config default (if one is ever added for another ollama-family entry) taking precedence over the reasoning-model fallback.uv run pytest tests/test_ollama_reasoning_effort.py tests/test_llm_backends.py tests/test_llm_parser.py tests/test_llm_parser_reasoning.py -q-- 133 passed.uv run pytest tests/ -q-- same 18 pre-existing failures as an unmodifiedv8checkout at this depth (shallow-clone git-history gaps intest_skillgen, environment-dependenttest_ollama*/test_ollama_retry_capfailures) confirmed by running the identical command againstgit stash-ed clean tree before this diff; this change adds zero new failures.Happy to adjust the default (e.g. a different effort level, or gating it behind an opt-in flag instead of a name-matched default) if you'd rather ship it more conservatively.