fix(shim): don't infer Z.AI tool_stream for non-catalog GLM gateways - #1908
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (3)
🧰 Additional context used📓 Path-based instructions (5)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**⚙️ CodeRabbit configuration file
Files:
**/*⚙️ CodeRabbit configuration file
Files:
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}⚙️ CodeRabbit configuration file
Files:
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (3)
📝 WalkthroughWalkthroughThe GLM openai-shim inference path now forces ChangesGLM tool streaming inference fix
Estimated code review effort: 1 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
please fix failing tests |
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
I found one repository-state blocker that needs to be addressed before this is ready.
Findings
- [P1] Fix the failing smoke-and-tests check before this is ready
Smoke / smoke-and-tests
GitHub currently reportssmoke-and-tests (24.11.x)failing in theSmoke and full unit test suitestep, and thesmoke-and-tests (22)matrix leg was cancelled after that failure. Since this PR changes the runtime metadata test surface and a reviewer has already asked for the failing tests to be fixed, please get the smoke-and-tests job green or update the PR with evidence that the failure is unrelated to this change before this can be treated as ready.
The name-based shim matcher inferred the full Z.AI GLM contract — including enableToolStreaming — for any glm-<n> model without a catalog entry. tool_stream is a Z.AI-proprietary streaming extension, so serving GLM through an arbitrary OpenAI-compatible gateway (e.g. NVIDIA NIM, integrate.api.nvidia.com) made every request fail immediately with 400 Unsupported parameter(s): tool_stream. Only a catalog entry may opt into tool_stream (Z.AI-contract gateways set it explicitly via transportOverrides.openaiShim). Inferred GLM routes keep the reasoning-shaping fields, which any GLM endpoint benefits from, but no longer send tool_stream; without it tool calls are simply not streamed.
8b6250f to
b956637
Compare
|
Thanks @jatmn. Dug into the smoke-and-tests failure — it's |
|
Tested this against the real-world bug (#1896). Environment: Samsung, Exynos 9820, custom ROM, Android 15 (API 35), Termux/aarch64, Node v26.3.1, npm 11.18.0, @gitlawb/openclaude 0.23.0 Instead of gating on "no catalog entry", I scoped the disable directly at the tool_stream assignment site: const isNvidiaNimEndpoint = request.baseUrl.includes('nvidia') Confirmed working end-to-end against NVIDIA NIM with z-ai/glm-5.2 (multi-step tool-call task, no tool_stream in the outgoing body). One thing worth considering: this PR's approach (disable when there's no catalog entry) is scoped to inference-path GLM models. A future catalog entry for GLM on a non-Z.AI gateway with enableToolStreaming: true would still slip through the "no catalog entry" check - the baseUrl-based gate covers that case too. Might be worth combining both. |
jatmn
left a comment
jatmn
left a comment
There was a problem hiding this comment.
Thanks for the update. I rechecked the previously discussed paths and do not see any remaining actionable issues from my side.
@kevincodex1 LGTM
…wigpine#1908) The name-based shim matcher inferred the full Z.AI GLM contract — including enableToolStreaming — for any glm-<n> model without a catalog entry. tool_stream is a Z.AI-proprietary streaming extension, so serving GLM through an arbitrary OpenAI-compatible gateway (e.g. NVIDIA NIM, integrate.api.nvidia.com) made every request fail immediately with 400 Unsupported parameter(s): tool_stream. Only a catalog entry may opt into tool_stream (Z.AI-contract gateways set it explicitly via transportOverrides.openaiShim). Inferred GLM routes keep the reasoning-shaping fields, which any GLM endpoint benefits from, but no longer send tool_stream; without it tool calls are simply not streamed.
T1 cherry-pick landed 8 upstream commits (Twigpine#1908 shim, Twigpine#1916 diff, Twigpine#1917 hunks, Twigpine#1913 powershell, Twigpine#1905 plan mode, Twigpine#1906 API cleanup, Twigpine#1901 content, Twigpine#1932 gitdiff cap). The following typecheck fixes were needed because OpenCC has stricter types than upstream (per `cherry-pick-test-localization-not-shipped`): 1. descriptors.ts:64 -- `enableToolStreaming?: true` widened to `boolean` so upstream's `enableToolStreaming: false` (in shim 2259c80) typechecks 2. apiTransform.ts:33 -- `message.uuid ?? ''` (UserMessage.uuid is optional in OpenCC Message type; upstream's UserMessage.uuid is required) 3. apiTransform.ts:225-261 -- `stripCallerFieldFromAssistantMessage` early-returns for string content; Upstream assumed `content` was array 4. content.ts:77-119 -- guard for OpenCC's optional `Message.message` and widened param types to accept `UserMessage` (uuid is `string | undefined`) 5. compact.test.ts:363 -- preserves OpenCC's existing `getAssistantMessageText` mock instead of upstream's "_realMessagesModule" (per user directive) Verification: - bun run typecheck: 0 errors (was 8 with cherry-picks+before-fix; 0 at baseline pre-T1) - bun test (T1 areas): 272 pass / 21 fail (21 fails are pre-existing GLM-5.2/estimateMessageTokens) - bun test (full): 4858 pass / 131 fail (same as baseline pre-T1; no regression) No rebrand or provider-policy changes needed; cherry-picks are 3-Provider-clean.
4 participants
Closes #1896.
Problem
Serving a GLM model (
z-ai/glm-5.2) through a third-party OpenAI-compatible gateway such as NVIDIA NIM (https://integrate.api.nvidia.com/v1) fails on the first turn:inferRemoteModelOpenAIShimConfig(inruntimeMetadata.ts) infers the fullZAI_GLM_OPENAI_SHIM— includingenableToolStreaming: true— for anyglm-<n>model that has no catalog entry.tool_streamis a Z.AI-proprietary streaming extension, but the matcher applied it from the model name alone, independent of which gateway is actually serving the model. Any non-Z.AI gateway then rejects the request.Fix
Keep inferring the GLM reasoning-shaping fields (preserved/echoed reasoning content,
zai-compatiblethinking format,max_tokens, strippingstore) — those help GLM on any endpoint — but stop inferringenableToolStreaming. Only a catalog entry may opt into it: the Z.AI-contract gateways (zai, opencode-go, atlas-cloud, hicap) already set it explicitly viatransportOverrides.openaiShim, so they are unaffected (their requests still sendtool_stream).This is host-agnostic on purpose — it fixes hosted NIM, self-hosted NIM, and any other custom OpenAI-compatible GLM endpoint, matching the report’s "breaks any non-Z.AI-official gateway".
tool_streamis only a streaming optimization; without it tool calls still work, just not streamed (the shim already has adelete body.tool_streamfallback for tool-incompatible retries).Test
Added a
resolveOpenAIShimRuntimeContextcase forz-ai/glm-5.2on the NIM base URL: asserts no catalog entry (inference path), the reasoning fields still apply, andenableToolStreamingis nottrue. Verified fail-on-main (the case fails without the change,enableToolStreaming === true). FullruntimeMetadatasuite (36) and theopenaiShimtool_streamcases (Z.AI path still sends it) pass;tsc --noEmitclean for the touched files.Distinct from #1892 (that covers
chat_template_kwargs/ reasoning on NIM); this is only thetool_streamrejection.Summary by CodeRabbit