fix(api): self-heal tool_stream rejection from non-Z.AI gateways (#1950) - #1951
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 31 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds ChangesTool-stream compatibility recovery
Claude stream watchdog test isolation
Estimated code review effort: 3 (Moderate) | ~25 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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/api/openaiErrorClassification.ts (1)
182-191: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle structured
Invalid parameterresponses.The matcher recognizes
invalidfor inline messages, but Lines 188-189 exclude it from theparam:tool_streamJSON patterns. A body such as{"error":{"message":"Invalid parameter","param":"tool_stream"}}therefore falls through without triggering recovery. Addinvalidto those alternatives and cover it with a regression test.As per path instructions, behavior changes require focused regression coverage.
Proposed fix
- /(?:unsupported|unknown|unrecognized|not\s+supported).*?\bparam(?:eter)?\s*[:=]\s*tool_stream\b/.test(normalized) || - /\bparam(?:eter)?\s*[:=]\s*tool_stream\b.*?(?:unsupported|unknown|unrecognized|not\s+supported)/.test(normalized) || + /(?:unsupported|unknown|unrecognized|invalid|not\s+supported).*?\bparam(?:eter)?\s*[:=]\s*tool_stream\b/.test(normalized) || + /\bparam(?:eter)?\s*[:=]\s*tool_stream\b.*?(?:unsupported|unknown|unrecognized|invalid|not\s+supported)/.test(normalized) ||🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/api/openaiErrorClassification.ts` around lines 182 - 191, Update the structured parameter patterns in the tool_stream classification logic to include “invalid” alongside the existing unsupported/unknown/unrecognized/not supported alternatives. Add focused regression coverage for a structured response with message “Invalid parameter” and param “tool_stream”, verifying it triggers recovery.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/services/api/openaiErrorClassification.ts`:
- Around line 182-191: Update the structured parameter patterns in the
tool_stream classification logic to include “invalid” alongside the existing
unsupported/unknown/unrecognized/not supported alternatives. Add focused
regression coverage for a structured response with message “Invalid parameter”
and param “tool_stream”, verifying it triggers recovery.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5fb05f11-ebd6-406d-8a7d-5185f966893f
📒 Files selected for processing (2)
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: typecheck
- GitHub Check: smoke-and-tests (22)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
Files:
src/services/api/openaiErrorClassification.tssrc/services/api/openaiErrorClassification.test.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/openaiErrorClassification.tssrc/services/api/openaiErrorClassification.test.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/openaiErrorClassification.tssrc/services/api/openaiErrorClassification.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/openaiErrorClassification.test.ts
🔇 Additional comments (2)
src/services/api/openaiErrorClassification.ts (1)
14-14: LGTM!Also applies to: 48-48, 152-168, 170-180, 513-516
src/services/api/openaiErrorClassification.test.ts (1)
151-168: LGTM!Also applies to: 170-177, 179-195, 197-204, 206-213, 215-228, 230-237, 239-246, 248-255, 257-264
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/services/api/openaiErrorClassification.test.ts (1)
161-168: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake the precedence test match both classifiers.
The body
Invalid parameter tool_stream for this tool_call requestverifies the new matcher, but it does not contain the existingtool_calls are not supportedsignal from the tool-compatibility test. It therefore may not catch a regression in the intended early-return precedence.- body: 'Invalid parameter tool_stream for this tool_call request', + body: 'Invalid parameter tool_stream; tool_calls are not supported by this model',As per path instructions, tests must provide meaningful coverage of the changed behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/api/openaiErrorClassification.test.ts` around lines 161 - 168, The precedence test for classifyOpenAIHttpFailure should use a response body containing both the tool_stream rejection signal and the existing “tool_calls are not supported” compatibility signal. Keep the expected category as tool_stream_unsupported so the test verifies the tool_stream classifier takes precedence when both classifiers match.Source: Path instructions
src/services/api/openaiErrorClassification.ts (1)
179-190: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winNarrow the
param=tool_streamfallback to avoid schema false positives.For example,
Invalid schema: param=tool_streammatches Line 189, so it is classified as an unsupported request parameter even though it can describe a schema/property failure. The existing exclusion only covers messages containingfunction/toolbeforeschema/properties; add coverage for generic schema contexts and narrow theinvalidfallback to the provider’s actual parameter-error format. Otherwise the downstream self-heal retry can mask the real schema error.As per path instructions, provider integration changes require high scrutiny for hidden fallback expansion and silent behavior changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/api/openaiErrorClassification.ts` around lines 179 - 190, The param=tool_stream fallback in the tool_stream error classifier is too broad and misclassifies generic schema failures such as “Invalid schema: param=tool_stream.” Update the relevant regex branches in the classifier to exclude generic schema/property contexts and restrict “invalid” matching to the provider’s confirmed parameter-error format, preserving unsupported/unknown parameter detection without expanding self-heal retries for schema errors.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/services/api/openaiErrorClassification.test.ts`:
- Around line 161-168: The precedence test for classifyOpenAIHttpFailure should
use a response body containing both the tool_stream rejection signal and the
existing “tool_calls are not supported” compatibility signal. Keep the expected
category as tool_stream_unsupported so the test verifies the tool_stream
classifier takes precedence when both classifiers match.
In `@src/services/api/openaiErrorClassification.ts`:
- Around line 179-190: The param=tool_stream fallback in the tool_stream error
classifier is too broad and misclassifies generic schema failures such as
“Invalid schema: param=tool_stream.” Update the relevant regex branches in the
classifier to exclude generic schema/property contexts and restrict “invalid”
matching to the provider’s confirmed parameter-error format, preserving
unsupported/unknown parameter detection without expanding self-heal retries for
schema errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2d93e975-21ce-4fb5-8535-67a9de96c50b
📒 Files selected for processing (2)
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: typecheck
- GitHub Check: smoke-and-tests (22)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/openaiErrorClassification.test.ts
🔇 Additional comments (3)
src/services/api/openaiErrorClassification.ts (2)
14-14: LGTM!Also applies to: 48-48
507-527: LGTM!src/services/api/openaiErrorClassification.test.ts (1)
151-160: LGTM!Also applies to: 170-177, 179-196, 198-205, 207-214, 216-229, 231-238, 240-247, 249-274
da2ff2b to
712fb7c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/api/openaiShim.test.ts`:
- Around line 8830-8894: Add a focused regression test alongside the existing
`#1950` self-heal test that makes both the initial request and the retry without
tool_stream return the same 400 error, then asserts client.beta.messages.create
rejects and exactly two request bodies were recorded. Reuse the existing tool
request setup and verify didRetryWithoutToolStream prevents any further retry or
loop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8e8e0ea7-c43d-40db-a6d0-d6be42bd2ba2
📒 Files selected for processing (7)
src/integrations/runtimeMetadata.test.tssrc/services/api/errors.openaiCompatibility.test.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: typecheck
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
Files:
src/services/api/errors.openaiCompatibility.test.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/errors.tssrc/services/api/openaiShim.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiShim.test.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/errors.openaiCompatibility.test.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/errors.tssrc/services/api/openaiShim.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiShim.test.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/errors.openaiCompatibility.test.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/errors.tssrc/services/api/openaiShim.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiShim.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/errors.openaiCompatibility.test.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.ts
🔇 Additional comments (10)
src/services/api/openaiErrorClassification.ts (4)
14-14: LGTM!Also applies to: 48-48
152-189: LGTM!
531-551: LGTM!
195-218: 🎯 Functional CorrectnessReview the
tool_streamsuppression guard
/\btool_stream\b.*?\b(?:tools|function|schema|properties?)\b/is broader than the other fallback checks and may suppress realtool_streamrejections that mention tools/function/schema later in the same message.src/services/api/openaiErrorClassification.test.ts (1)
151-309: LGTM!src/services/api/errors.ts (1)
151-156: LGTM!src/services/api/errors.openaiCompatibility.test.ts (1)
132-146: LGTM!src/integrations/runtimeMetadata.test.ts (1)
206-227: LGTM!src/services/api/openaiShim.ts (1)
4491-4492: LGTM!Also applies to: 4597-4597, 4683-4684, 4951-4980
src/services/api/openaiShim.test.ts (1)
4114-4144: LGTM!Also applies to: 8777-8828, 8895-8943
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/api/openaiErrorClassification.ts`:
- Around line 203-207: Correct the final tool_stream context guard in the
error-classification expression so function/tool call and calling contexts
remain excluded after whitespace matching, while allowing the optional article
“the” before a function or tool name. Add regression cases covering “in function
calls” and “in the function Bash”, preserving classification of genuine
top-level parameter errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: aeb96e02-b869-4542-a282-5aeb1ae4f9cb
📒 Files selected for processing (3)
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiShim.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
Files:
src/services/api/openaiErrorClassification.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/openaiErrorClassification.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/openaiErrorClassification.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.ts
🔇 Additional comments (4)
src/services/api/openaiErrorClassification.ts (1)
14-14: LGTM!Also applies to: 48-48, 535-555
src/services/api/openaiErrorClassification.test.ts (1)
151-168: LGTM!Also applies to: 170-177, 179-203, 205-212, 214-221, 223-237, 239-246, 248-255, 257-264, 266-278, 280-287, 289-297, 299-306, 308-315
src/services/api/openaiShim.test.ts (2)
8908-8930: LGTM!
8896-8906: 🩺 Stability & AvailabilityNo cleanup issue here shared
beforeEach/afterEachalready restoresOPENAI_BASE_URL,OPENAI_API_KEY, andglobalThis.fetch, so this test is isolated.> Likely an incorrect or invalid review comment.
34bf69d to
b980811
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/integrations/runtimeMetadata.test.ts`:
- Line 219: Strengthen the assertion for
result.openaiShimConfig.enableToolStreaming in the relevant runtime metadata
test by requiring it to be exactly false with a toBe(false) check. Replace the
weaker negated true assertion and leave the surrounding test behavior unchanged.
In `@src/services/api/openaiErrorClassification.ts`:
- Around line 152-221: Update getStructuredToolStreamValidationError so a root
["body", "tool_stream"] detail with a message indicating tool_stream is
unsupported is classified as true, rather than false; preserve false for
unrelated tool/schema validation errors. Add a regression test covering this
structured JSON msg variant and verify isToolStreamUnsupportedMessage identifies
it as unsupported.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: efd48c26-4e22-4f67-81dd-9747426f95ae
📒 Files selected for processing (7)
src/integrations/runtimeMetadata.test.tssrc/services/api/errors.openaiCompatibility.test.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: smoke-and-tests (22)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Run type validation for TypeScript changes usingbun run typecheckand, where relevant,bun run typecheck:type-tests.
Run focused tests for affected TypeScript test files and use the relevant provider test commands when provider behavior changes.
Files:
src/services/api/errors.openaiCompatibility.test.tssrc/services/api/errors.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Keep changes focused on one problem or clearly scoped feature, and avoid unrelated cleanup or broad rewrites.
Preserve existing repository patterns unless intentionally refactoring them.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary changes, and adherence to project architecture before submitting them.
Follow the existing code style in touched files; keep comments useful and concise; do not reformat unrelated files.
Provider changes must avoid breaking third-party providers while fixing first-party behavior, explicitly identify affected providers, test the exact changed provider/model path when possible, and document limitations or follow-up work.
Do not assign or use provider tags in provider-change pull requests; maintainers control those tags.
Dependency changes require a concrete project benefit, such as fixing a bug, addressing a security issue, or supporting an approved feature.
Run the narrowest meaningful validation command for the touched area before opening a pull request; pull requests must pass required CI checks.
Runbun run security:pr-scanbefore submitting changes when the pull request requires the project’s intent/security scan.
Files:
src/services/api/errors.openaiCompatibility.test.tssrc/services/api/errors.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/errors.openaiCompatibility.test.tssrc/services/api/errors.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when a code change affects behavior.
Files:
src/services/api/errors.openaiCompatibility.test.tssrc/services/api/errors.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/errors.openaiCompatibility.test.tssrc/services/api/errors.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiShim.tssrc/services/api/openaiShim.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/errors.openaiCompatibility.test.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiShim.test.ts
🔇 Additional comments (14)
src/services/api/openaiShim.test.ts (5)
8895-8930: LGTM — resolves the previously-flagged coverage gap.This is the "retry also fails" regression test requested in an earlier review round; it confirms exactly two attempts are made and no infinite loop occurs when the self-heal retry also returns the same error.
4114-4144: LGTM.Correctly asserts each pooled key is attempted exactly once (
maxAttempts = max(1, poolSize) = 2) with no reuse of a cooled-down key across the two 429 responses.
8777-8828: LGTM.Confirms
tool_streamis never sent on the NVIDIA NIM + Z.AI GLM path, matching the runtime gating asserted inruntimeMetadata.test.ts.
8836-8893: LGTM.Exercises the full self-heal happy path: first request carries
tool_stream: true, the retry drops only that field while preservingtools, and the request ultimately succeeds.
8932-8980: LGTM.Validates that the self-heal retry reuses the same pooled credential (
key-aon both attempts) rather than rotating to the next pool entry, matching theretryCredentialLeasebehavior inopenaiShim.ts.src/integrations/runtimeMetadata.test.ts (1)
206-227: LGTM! Regression test matches the upstream gating contract.The test correctly exercises
resolveOpenAIShimRuntimeContextfor NVIDIA NIM +z-ai/glm-5.2and confirmsenableToolStreamingis disabled while the reasoning-shaping fields (thinkingRequestFormat,preserveReasoningContent,requireReasoningContentOnAssistantMessages,maxTokensField,removeBodyFields) survive the override, matching the{ ...ZAI_GLM_OPENAI_SHIM, enableToolStreaming: false }contract inruntimeMetadata.ts.src/services/api/openaiShim.ts (4)
4491-4492: LGTM.New one-time-retry flag and carried-forward credential lease state, scoped correctly per request.
4593-4599: LGTM.Making
maxAttemptsmutable is the minimal change needed to let the new self-heal branch reserve an extra attempt without disturbing the existing GitHub/credential-pool budget math.
4682-4684: LGTM — reuse-then-clear pattern is correct.
retryCredentialLeaseis consumed and immediately reset tonulleach iteration, so the pooled-credential reuse only applies to the single retry iteration that requested it; subsequent iterations fall back tocredentialPool?.next()as before. Traced against thetool_stream_unsupportedbranch and the pooled-credential regression test (openaiShim.test.tslines 8932-8980) — behavior matches.
4951-4980: LGTM — self-heal branch is well-guarded.The
tool_stream_unsupportedbranch is gated by both!didRetryWithoutToolStreamandbody.tool_stream === true, giving double protection against looping if the retry itself somehow re-triggers this category. Budget growth (maxAttempts += 1) is scoped to only fire when the recovery is actually needed, and the credential lease is deliberately pinned to the one that received the rejection rather than rotated — correct, since this isn't a credential-related failure and rotating here could turn a benign 400 into a spurious auth failure. Consistent with the existingtool_call_incompatibleself-heal pattern just above it.src/services/api/openaiErrorClassification.ts (1)
1-221: Everything else here checks out.Traced the remaining regex branches and the new
classifyOpenAIHttpFailurebranch (533-553) against all provided positive/negative test cases by hand — all resolve correctly, including the previously-flaggedcalls?/article guard (now fixed at lines 204-205) and the ordering relative tocontext_overflow/tool_call_incompatible.Also applies to: 402-601
src/services/api/openaiErrorClassification.test.ts (1)
151-358: LGTM!src/services/api/errors.ts (1)
151-156: LGTM!src/services/api/errors.openaiCompatibility.test.ts (1)
132-146: LGTM!
…1950) GLM-5.2 served through NVIDIA NIM (`integrate.api.nvidia.com`) is rejected with `400 Unsupported parameter(s): tool_stream` because `tool_stream` is a Z.AI-proprietary streaming extension. Changes: - Add a `tool_stream_unsupported` OpenAI-compatibility failure category that detects the `tool_stream` rejection (the existing `tool_call_incompatible` matcher only matches `tool_call`, not `tool_stream`). - Self-heal in the OpenAI shim: when a gateway rejects `tool_stream`, drop only that parameter and retry with tools intact (streaming tool calls simply aren't streamed on such gateways). This is a defensive net for any provider that slips the parameter through, alongside the existing catalog/runtime gating that suppresses it for non-Z.AI routes. - Give remote requests one self-heal attempt budget so the retry can run. - Surface a friendly assistant message for the new category. Tests: - Regression test: NVIDIA NIM GLM streaming+tools never sends `tool_stream`. - Regression test: shim self-heals a `tool_stream` 400 by retrying without it. - Classification tests for `tool_stream_unsupported`. - Runtime metadata regression test for NVIDIA NIM GLM-5.2 (no `tool_stream`, reasoning shim preserved).
6669b8f to
f43589a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/api/errors.ts`:
- Around line 151-155: Update the error message in the tool_stream_unsupported
case of createAssistantAPIErrorMessage to say “switch models” instead of “switch
providers,” matching the /model or --model command represented by switchCmd.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 646e7912-1aac-4229-867c-ea903c7c280a
📒 Files selected for processing (7)
src/integrations/runtimeMetadata.test.tssrc/services/api/errors.openaiCompatibility.test.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.test.tssrc/services/api/openaiErrorClassification.tssrc/services/api/openaiShim.test.tssrc/services/api/openaiShim.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: typecheck
- GitHub Check: smoke-and-tests (22)
- GitHub Check: smoke-and-tests (24.11.x)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript code in this repository must use strict mode and ESM imports.
**/*.{ts,tsx}: Run type validation for TypeScript changes usingbun run typecheckand, where relevant,bun run typecheck:type-tests.
Run focused tests for affected TypeScript test files and use the relevant provider test commands when provider behavior changes.
Files:
src/services/api/errors.openaiCompatibility.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiErrorClassification.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.test.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/openaiShim.test.ts
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Keep changes focused on one problem or clearly scoped feature, and avoid unrelated cleanup or broad rewrites.
Preserve existing repository patterns unless intentionally refactoring them.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary changes, and adherence to project architecture before submitting them.
Follow the existing code style in touched files; keep comments useful and concise; do not reformat unrelated files.
Provider changes must avoid breaking third-party providers while fixing first-party behavior, explicitly identify affected providers, test the exact changed provider/model path when possible, and document limitations or follow-up work.
Do not assign or use provider tags in provider-change pull requests; maintainers control those tags.
Dependency changes require a concrete project benefit, such as fixing a bug, addressing a security issue, or supporting an approved feature.
Run the narrowest meaningful validation command for the touched area before opening a pull request; pull requests must pass required CI checks.
Runbun run security:pr-scanbefore submitting changes when the pull request requires the project’s intent/security scan.
Files:
src/services/api/errors.openaiCompatibility.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiErrorClassification.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.test.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/openaiShim.test.ts
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/services/api/errors.openaiCompatibility.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiErrorClassification.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.test.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/openaiShim.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when a code change affects behavior.
Files:
src/services/api/errors.openaiCompatibility.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiErrorClassification.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.test.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/openaiShim.test.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}
⚙️ CodeRabbit configuration file
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.
Files:
src/services/api/errors.openaiCompatibility.test.tssrc/services/api/openaiShim.tssrc/services/api/openaiErrorClassification.tssrc/services/api/errors.tssrc/services/api/openaiErrorClassification.test.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/openaiShim.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/services/api/errors.openaiCompatibility.test.tssrc/services/api/openaiErrorClassification.test.tssrc/integrations/runtimeMetadata.test.tssrc/services/api/openaiShim.test.ts
🔇 Additional comments (5)
src/services/api/openaiErrorClassification.ts (1)
14-14: LGTM!Also applies to: 48-48, 152-231, 544-564
src/services/api/openaiErrorClassification.test.ts (1)
151-372: LGTM!src/integrations/runtimeMetadata.test.ts (1)
202-224: LGTM!src/services/api/openaiShim.ts (1)
4491-4492: LGTM!Also applies to: 4597-4599, 4683-4684, 4951-4980
src/services/api/openaiShim.test.ts (1)
4114-4144: LGTM!Also applies to: 8777-8930, 8932-8980
2 participants
Summary
Fixes #1950: GLM-5.2 served through NVIDIA NIM (
integrate.api.nvidia.com) is rejected with400 Unsupported parameter(s): tool_stream.tool_streamis a Z.AI-proprietary streaming extension that non-Z.AI gateways don't accept.Root cause
The
tool_streamparameter is only sent for Z.AI-contract routes (zai / hicap / opencode-go / atlas-cloud), which opt in explicitly via catalogtransportOverrides. The runtime/config gating already suppresses it for third-party GLM gateways (see #1908). However, the specific400 Unsupported parameter(s): tool_streamerror was not detected by the existingtool_call_incompatiblematcher (it matchestool_call, nottool_stream), so no self-heal triggered and the user got a hard failure.Changes
tool_stream_unsupportedinopenaiErrorClassification.tsthat detectstool_streamrejections (matchestool_stream+ an unsupported/unknown/invalid-parameter signal).tool_stream, drop only that parameter and retry with tools intact. Streaming tool calls simply aren't streamed on such gateways — the request still succeeds.0for remote), so the retry can actually run.errors.ts.Tests
tool_stream(the exact GLM 5.2 Not working NVIDIA NIMtool_streamNot supported #1950 path — was previously untested at the end-to-end level).tool_stream400 by retrying without it, preserving tools.tool_stream_unsupported(positive + negative).enableToolStreamingoff while keeping the GLM reasoning shim.Validation
bun test ./src/services/api/openaiShim.test.ts— 218 passbun test ./src/services/api/openaiErrorClassification.test.ts ./src/integrations/runtimeMetadata.test.ts— 66 passbun run typecheck— clean(
bun run checkhas a pre-existingknipfailure on unusedundici/wsdevDependencies that is unrelated to this change and reproduces on a cleanmain.)🤖 Generated with OpenCode
Summary by CodeRabbit
tool_stream: retries once withouttool_stream, keeps existing tool configuration, and reuses the same pooled credential.glm-5.2, ensuringtool_streamremains disabled and reasoning-shaping fields are set correctly.tool_stream-unsupported errors with clear “switch models” assistant guidance (no misleading retry text).tool_streamrejection detection, assistant guidance output, and single self-heal behavior.