Skip to content

fix(transports): normalize xhigh and minimal reasoning effort levels to high and low - #85334

Closed
finansovazashtita-arch wants to merge 1 commit into
NousResearch:mainfrom
finansovazashtita-arch:fix/reasoning-xhigh-clamping
Closed

fix(transports): normalize xhigh and minimal reasoning effort levels to high and low#85334
finansovazashtita-arch wants to merge 1 commit into
NousResearch:mainfrom
finansovazashtita-arch:fix/reasoning-xhigh-clamping

Conversation

@finansovazashtita-arch

Copy link
Copy Markdown

Fixes HTTP 400 error (invalid reasoning value 'xhigh') when reasoning effort is set to xhigh on OpenAI-compatible and Nous Portal endpoints. Clamps xhigh -> high and minimal -> low before assembling reasoning extra_body.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/openai OpenAI / Codex Responses API provider/nous Nous Research API (OAuth) labels Aug 13, 2026
@spfcraze

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary:
The diff also changes copilot credential seeding: agent/credential_pool.py now returns early from _seed_from_env for the copilot provider, a behavior change the reasoning-effort description does not describe.

Problems:

  • agent/credential_pool.py:1383 adds if provider == "copilot": return changed, active_sources at the top of _seed_from_env. Copilot's registry entry (hermes_cli/auth.py:185) is auth_type="api_key" with api_key_env_vars=("COPILOT_GITHUB_TOKEN", "GH_TOKEN", "GITHUB_TOKEN"), so the generic env-seed path would otherwise seed a copilot pool entry from those env vars; the early return stops that.
  • This is unrelated to the PR's stated purpose (normalizing xhigh/minimal reasoning effort), and the body does not mention it.

Solution:
Split the copilot seeding change into its own PR, or describe it in the body so the reasoning-effort fix is reviewed on its own scope.

Evidence

no deterministic fact backs this claim — model belief, not executed or read evidence


Checked against 5e58c92 — the PR head when this was written — and fa83af3, main at the same moment.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(transports): normalize xhigh and minimal reasoning effort levels to high and low

  1. Clamping logic triplicated — the minimal→low / xhigh→high mapping is inlined three times in different shapes: an if/elif in chat_completions.py::build_kwargs, a dict in codex.py::build_kwargs, and another if/elif in run_agent.py::_handle_max_iterations. Extract one shared helper (e.g. in a transports util) so the three sites cannot drift; the codex dict and the two if/elif chains already differ in shape.
  2. Silent lossy mapping — a user explicitly choosing minimal gets low and xhigh gets high with no signal. Consider a logger.debug when clamping, and/or validating the effort enum at config-load time so an out-of-range value surfaces once instead of being silently remapped at every call site.
  3. Clamp applies beyond the intended provider — in chat_completions.py the normalization sits inside the else of the enabled is False check, i.e. it applies to every provider routed through extra_body["reasoning"], not just Nous. Confirm the target providers only accept low/medium/high; otherwise a provider that genuinely supports xhigh is silently downgraded.
  4. Shallow copy in run_agent.pyrc_sum = dict(self.reasoning_config) is fine for the effort mutation today, but any future nested-key mutation would alias the original config. Either note the constraint or use a deep copy.

@teknium1

Copy link
Copy Markdown
Contributor

The premises here don't hold on current main, and one hunk would reintroduce a fixed bug class:

  1. xhigh → high on the chat-completions wire: xhigh is a valid wire level on the major OpenAI-compatible routes (Nous Portal and OpenRouter both accept max|xhigh|high|..., verified live) — blanket-downgrading it would silently cap models that support it. Routes with narrower vocabularies clamp via their declared sets in agent/reasoning_effort.py (PRs fix: reasoning effort 'ultra' no longer 400s on non-Anthropic wires (#89503 class, salvage #89509) #90330/refactor: reasoning-effort clamps can no longer drift per vendor — one canonical ladder + declared wire vocabularies #90350/fix: 'max' effort no longer 400s on pre-5.6 Codex Responses models (#68365 verified live) #90441).
  2. Codex xhigh → high: live-verified (Aug 2026): both gpt-5.6 and gpt-5.5 accept xhigh on the Responses API — the clamp isn't needed; minimal → low is already handled per-model there.
  3. The bundled copilot credential-pool _seed_from_env change is unrelated to effort normalization and would need its own PR with its own justification.

Closing — the effort-vocabulary class is centralized now, so narrow-vocabulary routes are a one-tuple declaration rather than transport-level rewrites. If you have a specific provider that rejects xhigh, please open an issue naming it and its accepted set.

@teknium1 teknium1 closed this Aug 20, 2026
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 P2 Medium — degraded but workaround exists provider/nous Nous Research API (OAuth) 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.

5 participants