Skip to content

fix(transports+agent): GLM reasoning_effort ultra + drop empty tool_calls key (#70058, #70126) - #70182

Closed
ms-alan wants to merge 2 commits into
NousResearch:mainfrom
ms-alan:fix/70058-70126-reasoning-tool-calls
Closed

fix(transports+agent): GLM reasoning_effort ultra + drop empty tool_calls key (#70058, #70126)#70182
ms-alan wants to merge 2 commits into
NousResearch:mainfrom
ms-alan:fix/70058-70126-reasoning-tool-calls

Conversation

@ms-alan

@ms-alan ms-alan commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Closes #70058
Closes #70126

Two fixes:

  1. reasoning_effort: "ultra" rejected by GLM API (HTTP 400), triggers silent fallback to Claude Sonnet #70058 - GLM reasoning_effort ultra:
    only checked for when normalizing
    to the wire-compatible . GLM models (glm-4, glm-4-plus,
    etc.) use the same parameter but were never normalized, causing the
    raw string 'ultra' to be sent instead of the expected integer.

  2. [Bug]: sanitize_api_messages dedup writes tool_calls: [], breaking Qwen-series tool-calling sessions on v0.18.2+ #70126 - drop empty tool_calls key: Step 3
    deduplication replaced with even when
    was (every tool_call had a duplicate id). Strict
    OpenAI-compatible providers (Qwen-series) reject
    with HTTP 400. Fix: when is empty, drop the
    key entirely instead of setting it to .

chenbin added 2 commits July 24, 2026 00:14
…upes

sanitize_api_messages() Step 3 deduplication creates tool_calls: []
when every tool_call in a turn has a duplicate id. This empty array
then passes through to strict OpenAI-compatible providers (Qwen-series)
which reject it with HTTP 400. Fix: when kept_tcs is empty after
dedup, drop the tool_calls key entirely instead of setting it to [].

Closes NousResearch#70126
…odels

GLM models (glm-4, glm-4-plus, etc.) use the same reasoning_effort:
ultra parameter as GPT-5.6 ultra. The _reasoning_config_for_model
function was only checking for gpt-5.6, causing GLM reasoning effort
to be sent as raw string 'ultra' instead of the normalized integer.
This fixes the mismatch for GLM model families.

Closes NousResearch#70058
@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/qwen Qwen / Alibaba Cloud (OAuth) provider/zai ZAI provider sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state needs-decision Awaiting maintainer decision before any implementation labels Jul 23, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #69917 covers the focused GLM-5.2 alias policy and #64345 removes the empty tool_calls key. This PR broadens the GLM policy, while its empty-list hunk leaves the original key unchanged when no calls remain; maintainers should choose the intended consolidation.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for combining two real provider-compatibility reports.

Problems

  • The agent/agent_runtime_helpers.py:2875 hunk does not remove the key when every call was deduplicated. if kept_tcs: leaves the original duplicate calls unchanged in that case, instead of preserving deduplication and dropping tool_calls. Current main's dedup contract is at agent/agent_runtime_helpers.py:3142-3167.
  • The GLM change is broader than the existing GLM-5.2 policy in plugins/model-providers/zai/__init__.py:49-82, and it misses the direct summary path: agent/chat_completion_helpers.py:2199-2206 forwards agent.reasoning_config without calling the transport helper.
  • The PR adds no regression tests; existing transport coverage only checks GPT-5.6 at tests/agent/transports/test_chat_completions.py:20-35.

Suggested changes

  • On a changed dedup list, retain non-empty calls or remove the tool_calls key when none survive; add the all-duplicate regression case.
  • Limit GLM normalization to verified GLM-5.2 aliases and apply the same normalizer to summary requests, with tests for both paths.

Automated hermes-sweeper review.

seen_assistant_call_ids.add(cid)
kept_tcs.append(tc)
if len(kept_tcs) != len(msg.get("tool_calls") or []):
if kept_tcs:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When every call is a duplicate, kept_tcs is empty and this leaves msg unchanged, including the original duplicate IDs. Please retain the changed-list check and remove the tool_calls key when no calls survive, so the sanitizer does not trade tool_calls: [] for duplicate-ID validation failures.

return reasoning_config
if (
"gpt-5.6" in (model or "").lower()
("gpt-5.6" in (model or "").lower() or "glm-" in (model or "").lower())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please scope this to verified GLM-5.2 aliases rather than every glm- model. The existing Z.AI profile only maps native reasoning effort for GLM-5.2 (plugins/model-providers/zai/__init__.py:49-82), and the direct summary path also needs the same normalization.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 30, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the report-and-fix. This added glm- next to the existing gpt-5.6 guard, but the bug class was broader than the two vendor prefixes — every model on the OpenAI-compatible transport rejects the internal ultra level. PR #90330 fixes it by translating ultra → max unconditionally at the chokepoint (plus sibling sites), which covers GLM without a per-vendor list. The bundled sanitize_api_messages change was unrelated to this bug and would need its own PR with its own justification if still relevant. Closing in favor of #90330.

@teknium1 teknium1 closed this Aug 19, 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 needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists provider/qwen Qwen / Alibaba Cloud (OAuth) provider/zai ZAI provider sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

3 participants