Accept every reasoning effort ReqLLM accepts, including max - #254
Merged
Merged
Conversation
Imp's accepted efforts are read from ReqLLM's own reasoning_effort option instead of a second list, so max is accepted at build, per call and in a saved program. A string effort reaches ReqLLM as its atom: ReqLLM's OpenRouter provider refused the string form.
… on Anthropic too
# Conflicts: # CHANGELOG.md
…guard The catch-all after ReqLLM.model/1's ok/error clauses could never match; an unexpected result is a CaseClauseError the function's rescue already turns into an error, so the clause and its ignore entry go. The provider_meta guard stays deliberately defensive and its ignore entry moves to the line it now covers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Imp.Clients.ReqLLMreads its accepted:reasoning_effortvalues from ReqLLM's own generation schema (ReqLLM.Provider.Options.generation_schema(), the{:in, [...]}type ofreasoning_effort) instead of keeping a second list, so Imp acceptsmax, which ReqLLM's list already had (since at least 1.18) and Imp's hand-written list lacked. The match fails the build if ReqLLM changes the option's shape.Imp.Saving) checks against the same list, so a saved"max"loads.reasoning_effortagainst its atom list before most providers see it; only OpenAI, xAI and Meta convert the string form first. So in 0.6.0 a string effort, including every effort loaded from a saved program (always a string), failed every call on OpenRouter's default top-level wire, Anthropic, Google and Groq, with a NimbleOptions error. OpenRouter's nested wire, OpenAI and xAI were not affected..dialyzer_ignore.exsentries pinned by line in this file. The catch-all afterReqLLM.model/1's{:ok, _}/{:error, _}clauses inresolve_model/1could never match (ReqLLM's contract); an unexpected result now raises a CaseClauseError that the function's ownrescueturns into{:error, _}, so the clause and its ignore entry are removed. Theresponse.provider_meta || %{}guard stays defensive (the struct does not enforce its map type); its entry is re-pinned to line 1902 with its existing reason.mix dialyzer.checkpasses locally.Why
Some model residents are configured for the highest thinking level. ReqLLM accepts
:max; Imp 0.6.0 refused it at build and per call.Decision: accept every effort ReqLLM accepts, on every provider, and leave the mapping to ReqLLM. I read how ReqLLM 1.24 handles
:maxper provider: OpenRouter, Zenmux, Groq, xAI, Mistral and other pass-through providers send"max"; OpenAI's param profiles send"max"; Anthropic maps to"max"where the model supports it, otherwise"xhigh"or a fixed budget; Google maps to:high/ 32768 tokens; LM Studio clamps toxhighwith a warning; Ollama and Fireworks send"max"; Moonshot uses:max. DeepSeek sends"max"as a string pass-through. None rejects it inside ReqLLM; whether a given model accepts"max"is the provider API's call, as it already is forxhigh.Tests
ReqLLMClientTest"reasoning_effort max reaches OpenRouter as "max" on either wire": a Req adapter (no network) captures the request body."max"configured on the client arrives as top-level"reasoning_effort": "max";:maxon a call over a client with no effort does the same;openrouter_reasoning_wire: :nestedsends"reasoning": {"effort": "max"}.ReqLLMClientTest"a string reasoning_effort reaches OpenRouter and Anthropic requests": the existing string"high"on OpenRouter's top-level wire arrives as"reasoning_effort": "high", and on Anthropic arrives as athinkingblock.Imp.SavingReqLLMTransportTest"saved reasoning effort round-trips and remains narrowly allowlisted" round-trips:maxthrough dump/load.:lowrather than"low"reaching the ReqLLM module, which is the new conversion.Falsification: with the old hard-coded list restored, the new OpenRouter test and the saving test fail (2 failures). With the string-to-atom conversion removed, the new OpenRouter test fails with ReqLLM's
invalid value for :reasoning_effort ... got: "max", and the native-reasoning test fails. Withlib/taken from main, the string-effort test fails on its OpenRouter half and, with that half removed, on its Anthropic half, both with ReqLLM'sgot: "high"validation error. All restored: pass.mix checkafter merging main (with #253) and the dialyzer fix: exit 0;59 doctests, 9 properties, 3545 tests, 0 failures, 13 skipped (221 excluded).Unsure / left alone
bench/imp/benchmark_truth/rlm_manifest.exkeeps its own effort list. That manifest is shared with the DSPy runtime, so ReqLLM's list is not obviously its authority; left unchanged.Imp.Clients.ReqLLM.reasoning_efforts/0stays@doc false; making it public so Dwell can drop its own list is a follow-up.