Skip to content

fix: emit reasoning_effort for custom OpenAI-compatible providers - #90057

Closed
LeonardoLGDS wants to merge 1 commit into
NousResearch:mainfrom
LeonardoLGDS:fix/custom-provider-reasoning-effort
Closed

LeonardoLGDS wants to merge 1 commit into
NousResearch:mainfrom
LeonardoLGDS:fix/custom-provider-reasoning-effort

Conversation

@LeonardoLGDS

Copy link
Copy Markdown
Contributor

Why

Issue #90031: for custom OpenAI-compatible providers (provider: custom, e.g. a local llama.cpp server via base_url), reasoning_effort was silently dropped. Custom providers do not advertise supports_reasoning, so the generic extra_body.reasoning arm never fires, and the server falls back to its model default (usually no reasoning or default effort), ignoring what the user configured.

What

Added a custom-scoped arm in ChatCompletionsTransport.build_kwargs() (agent/transports/chat_completions.py), after the LM Studio arm, that emits top-level reasoning_effort for custom providers only when the user explicitly set an effort (config.yaml agent.reasoning_effort or a session /reasoning override; both arrive resolved in the same reasoning_config slot). Levels are passed through identically for low/medium/high; unset, disabled (reasoning_effort: false), or levels outside the OpenAI wire set (xhigh, ultra, etc.) omit the field instead of inventing a value. No new config keys, no env vars, no supports_reasoning side effect, no extra_body usage. The existing Kimi / TokenHub / LM Studio arms and the generic extra_body.reasoning arm are untouched.

xhigh and friends remain reachable for users who need them via the existing extra_body route; the new arm deliberately stays within the OpenAI low|medium|high wire set.

Verification

Tests-first in tests/agent/transports/test_chat_completions.py (new class TestChatCompletionsCustomProviderReasoning):

  • Red run (tests only, no arm yet): 1 failed, 51 deselected with KeyError: 'reasoning_effort' on the first emission test.
  • After adding the arm: 8 passed (targeted), 59 passed (full transport file).
  • Sabotage check: temporarily removed only the new arm, targeted run went red again (1 failed, same missing-key error), restored byte-identically, both runs green again (8 passed, 59 passed).
  • Independent re-run (driver, separate from the drone): 59 passed in 2.57s.

Covered by the tests: custom+medium emits top-level; low/high identity pass-through (covers the session override slot, runtime beats config); unset omits (no reasoning_effort, no extra_body.reasoning); disabled omits; unsupported level omits; messages list byte-identical with the branch firing and not firing; negative: Kimi takes its own arm with no double emission.

Scope notes

This is complementary to #83566 (Ollama gate fix in run_agent.py, which does not cover provider: custom endpoints) and rebases cleanly either way. #87413 (reasoning_format surface) is still unmerged; if that design lands, the per-vendor arms are the natural seam to fold into it.

One known residual: the summary/compaction path in agent/chat_completion_helpers.py calls chat.completions.create() directly and mirrors only the LM Studio top-level emission and the supports_reasoning extra_body shape. For custom providers it never emitted reasoning_effort before this change and still does not; extending it is a separate, larger change and is deliberately out of scope here. Unknown-field residual risk on strict OpenAI-compatible gateways is unchanged from the existing Kimi/TokenHub top-level arms (the field is part of the OpenAI Chat Completions schema).

Fixes #90031
Refs #83566
Refs #87413

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/openai OpenAI / Codex Responses API labels Aug 19, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review; please use your judgment.

  1. agent/transports/chat_completions.py:~603–610 — the arm checks only that effort normalizes into the wire set; it never consults enabled. A config of {"enabled": False, "effort": "high"} — plausible for someone who turned thinking off but left the level configured — emits reasoning_effort: "high", silently re-enabling reasoning server-side, the exact class of surprise the "unset omits" test exists to prevent — why it matters: the existing disabled-case test uses {"enabled": False} with no effort key, so this combination ships untested and wrong — suggestion: require reasoning_config.get("enabled") before reading effort, and add the disabled-with-effort case to the matrix.

  2. Nit: the {"low", "medium", "high"} wire set is now spelled inline here while sibling arms define their own accepted sets nearby; hoisting one module-level constant (with a comment that it's the OpenAI wire vocabulary, not each provider's capability) keeps future level additions consistent across arms.

Otherwise clean: identity pass-through without inventing values, deliberate omission over defaults, no supports_reasoning side effects, the Kimi mutual-exclusion negative, and the messages-byte-identical assertion are all the right tests for this kind of request-shaping change.

— reviewer-a · automated agent review (Hermes week-review)

@cvaisnor

Copy link
Copy Markdown

I posted a repro on #90031 that I think affects the premise here, so flagging it rather than letting this merge quietly.

Two things:

1. Plain custom already emits top-level reasoning_effort. Not via the generic extra_body.reasoning arm — that one is indeed gated on supports_reasoning — but via the provider plugin: plugins/model-providers/custom/__init__.py:76 sets top_level["reasoning_effort"], and agent/transports/chat_completions.py:809 merges it with api_kwargs.update(top_level_from_profile), unconditionally and with no supports_reasoning gate. Checked on v0.20.5 / 41447a6d. That matches the request dump in #90031 and the recording-proxy table in #72202, both of which show plain provider: custom putting the field on the wire. Worth confirming whether this arm and the profile emit can ever disagree about the same key.

2. Restricting to low|medium|high would regress xhigh users. CustomProfile clamps against OPENAI_COMPAT_WIRE_EFFORTS, which includes xhigh, and local llama.cpp honours it — I verified on build 10573 that a top-level reasoning_effort: xhigh renders correctly. Omitting anything outside the OpenAI wire set would take that away from anyone running xhigh against a local server today.

Also worth weighing #72649 alongside this: it argues an unconditional top-level reasoning_effort on the heterogeneous custom bucket returns a non-retryable HTTP 400 on LiteLLM proxies. A second unconditional emit makes that harder to opt out of.

The gap that does still look real is #72202 — the named custom:<name> fallback losing its profile and sending nothing — which isn't covered here.

None of this is a knock on the implementation; the red-run-plus-sabotage-check discipline is better than most of what I read. I just don't think the root cause it's built on holds on current builds.

@LeonardoLGDS

Copy link
Copy Markdown
Contributor Author

Thanks @cvaisnor for the repro and detailed check.

You are correct that in v0.20.5+, CustomProfile in plugins/model-providers/custom/__init__.py already injects top_level["reasoning_effort"] without requiring supports_reasoning, and that clamping to low|medium|high would restrict endpoints supporting xhigh (such as local llama.cpp).

Since the main pain point was the named profile drop (#72202) and that is better addressed in the provider resolution chain, I'll close this in favor of keeping the existing profile emit intact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have provider/openai OpenAI / Codex Responses API type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

reasoning_effort is dropped for custom (OpenAI-compatible) providers — local llama.cpp falls back to model default

4 participants