Skip to content

fix(agent): clamp GLM-5.2 ultra reasoning effort - #69917

Closed
shaoohh wants to merge 1 commit into
NousResearch:mainfrom
shaoohh:agent/fix-glm52-ultra-effort
Closed

fix(agent): clamp GLM-5.2 ultra reasoning effort#69917
shaoohh wants to merge 1 commit into
NousResearch:mainfrom
shaoohh:agent/fix-glm52-ultra-effort

Conversation

@shaoohh

@shaoohh shaoohh commented Jul 23, 2026

Copy link
Copy Markdown

Summary

  • normalize ultra to GLM-5.2's supported max reasoning effort before provider-specific request shaping
  • cover canonical and relay GLM-5.2 model aliases, including the reported nvidia/z-ai/glm-5.2 route
  • preserve the existing reasoning configuration for unrelated model families

Testing

  • scripts/run_tests.sh tests/agent/transports/test_chat_completions.py -q (94 passed)
  • .venv/Scripts/ruff.exe check agent/transports/chat_completions.py tests/agent/transports/test_chat_completions.py

Fixes #69855

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/nvidia NVIDIA NIM provider/zai ZAI provider P2 Medium — degraded but workaround exists labels Jul 23, 2026
@GottZ GottZ mentioned this pull request Jul 28, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused fix. The normal Chat Completions premise is confirmed on current main: agent/transports/chat_completions.py:25-32 only clamps ultra for GPT-5.6, while the Nous profile forwards enabled reasoning configuration at plugins/model-providers/nous/__init__.py:60-68.

Problems

  • agent/chat_completion_helpers.py:2199-2206 builds session-summary Chat Completions requests directly and assigns agent.reasoning_config verbatim to summary_extra_body["reasoning"]; it bypasses ChatCompletionsTransport._reasoning_config_for_model(). A GLM-5.2 summary request can therefore still emit unsupported ultra after this PR.

Suggested changes

  • Reuse the same model-aware normalization before constructing the summary reasoning body, and add a regression test for that direct summary path.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the fix. The GLM-5.2 ultra leak is resolved on main: PR #90330 clamped ultra → max unconditionally at the chat-completions chokepoint, and PR #90350 replaced the per-vendor guard lists with a canonical effort ladder + declared per-route vocabularies (GLM-5.2's two-level high/max set included, shared by the zai and opencode-zen profiles) — so this can't regress per-vendor again. Your model-name-token approach was a reasonable extension of the pattern that existed at the time; the class fix just went one level deeper. Closing with thanks.

@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/nvidia NVIDIA NIM provider/zai ZAI provider sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

did a branch, failed

3 participants