feat(provider): add OpenAI-compatible backends (DeepSeek, Bailian, Moonshot, Zhipu) - #16
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughAdds support for OpenAI-compatible vendor gateways (DeepSeek, Qwen/DashScope, Moonshot/Kimi, Zhipu/GLM) by introducing an ChangesOpenAI-Compatible Gateway Integration
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI / API Runner
participant resolveModel
participant factory as factory.ts
participant OPENAI_COMPATIBLE as OPENAI_COMPATIBLE Registry
participant OpenAIProvider
CLI->>resolveModel: resolveModel("deepseek/deepseek-chat")
resolveModel->>OPENAI_COMPATIBLE: lookup "deepseek" prefix
OPENAI_COMPATIBLE-->>resolveModel: CompatibleBackend config
resolveModel-->>CLI: { kind: "deepseek", model: "deepseek-chat" }
CLI->>factory: hasCredentials("deepseek")
factory->>OPENAI_COMPATIBLE: firstPresentEnv(apiKeyEnv)
OPENAI_COMPATIBLE-->>factory: key present
factory-->>CLI: true
CLI->>factory: providerFor("deepseek/deepseek-chat")
factory->>OPENAI_COMPATIBLE: get baseURL, apiKeyEnv, maxTokensField
factory->>OpenAIProvider: new OpenAIProvider({ name:"deepseek", baseURL, apiKey, maxTokensField:"max_tokens" })
OpenAIProvider-->>CLI: provider instance
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
packages/provider/src/factory.test.ts (1)
58-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression cases for
toString/…and emptyvendor/overridesPlease extend this test block with edge cases that assert prototype-key prefixes and empty stripped models are rejected. This prevents regressions in override routing.
Suggested test additions
test("resolveModel honors a vendor/model override and strips the prefix", () => { @@ // An unknown prefix before "/" is NOT treated as an override. expect(() => resolveModel("mystery/thing")).toThrow(ValidationError); + // Prototype-chain key must not be accepted as a provider name. + expect(() => resolveModel("toString/thing")).toThrow(ValidationError); + // Empty model after a valid vendor prefix must be rejected. + expect(() => resolveModel("openai/")).toThrow(ValidationError); });🤖 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 `@packages/provider/src/factory.test.ts` around lines 58 - 70, The test block for resolveModel is missing regression cases for edge cases that should be rejected. Add additional test assertions to the test "resolveModel honors a vendor/model override and strips the prefix" that verify the resolveModel function throws ValidationError for prototype-key prefixes such as "toString/...", "constructor/...", and "__proto__/..." as well as for empty vendor overrides like "vendor/" with nothing after the slash. These should be added as expect calls similar to the existing mystery/thing case to ensure these dangerous inputs are properly rejected.
🤖 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 `@CLAUDE.md`:
- Around line 68-72: The documentation in CLAUDE.md describing supported
providers is incomplete and doesn't match the actual provider registry in
openai-compatible.ts. Update lines 70-71 to include the missing Bailian model
prefixes (qwq* and qvq*) alongside the existing qwen* entry, and add a note
about the GLM_API_KEY fallback support for the Zhipu models that is already
implemented in the provider registry. Ensure the documentation matrix accurately
reflects all model prefixes and environment variable fallbacks that are actually
supported in the code.
In `@packages/provider/src/factory.ts`:
- Around line 44-47: In the resolveModel function in the factory.ts file, after
validating that the prefix is a valid provider name using
isProviderName(prefix), add a validation check to ensure the model part (the
substring after the slash) is not empty. When modelId.slice(slash + 1) returns
an empty string, immediately throw a ValidationError with a descriptive message
indicating that the vendor override format requires a non-empty model name,
rather than allowing the empty model string to be returned and cause failures
later at request time.
- Around line 56-75: The construct function's default case creates an
OpenAIProvider for compatible backends, but when apiKey is undefined (returned
from firstPresentEnv call), the OpenAIProvider constructor may fall back to the
OPENAI_API_KEY environment variable, causing the OpenAI key to be sent to
third-party gateways. Before creating the OpenAI-compatible provider instance,
gate the construction on hasCredentials(kind) to verify that the required
credentials exist for the specified kind, and handle the case where credentials
are missing by throwing an appropriate error instead of allowing the fallback
behavior.
- Around line 17-19: The isProviderName function uses the `in` operator to check
if s is in OPENAI_COMPATIBLE, which accepts inherited properties from the
prototype chain like toString. Replace the `s in OPENAI_COMPATIBLE` check with
Object.hasOwn(OPENAI_COMPATIBLE, s) or
Object.prototype.hasOwnProperty.call(OPENAI_COMPATIBLE, s) to ensure only own
properties of the OPENAI_COMPATIBLE object are accepted, preventing
attacker-controlled inputs like "toString" from bypassing the guard and causing
crashes in downstream code that accesses properties on OPENAI_COMPATIBLE[s].
In `@packages/provider/src/openai-compatible.ts`:
- Around line 13-16: The interface CompatibleBackend declares apiKeyEnv and
prefixes as mutable arrays, but the registry entries at line 61 use as const
satisfies which creates readonly tuples, causing TypeScript to reject the
assignment. Add the readonly modifier to both the apiKeyEnv: string[] and
prefixes: RegExp[] properties in the CompatibleBackend interface to make them
readonly, which will match the readonly tuple types from the registry entries
and resolve the compilation error.
---
Nitpick comments:
In `@packages/provider/src/factory.test.ts`:
- Around line 58-70: The test block for resolveModel is missing regression cases
for edge cases that should be rejected. Add additional test assertions to the
test "resolveModel honors a vendor/model override and strips the prefix" that
verify the resolveModel function throws ValidationError for prototype-key
prefixes such as "toString/...", "constructor/...", and "__proto__/..." as well
as for empty vendor overrides like "vendor/" with nothing after the slash. These
should be added as expect calls similar to the existing mystery/thing case to
ensure these dangerous inputs are properly rejected.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2b2b7fa9-7c24-45b8-90c0-326e609cbdbd
📒 Files selected for processing (14)
CLAUDE.mdCONTRIBUTING.mdapps/api/src/runner.tsdocs/architecture.mdpackages/capella/src/cost.test.tspackages/capella/src/cost.tspackages/cli/src/main.tspackages/provider/src/factory.test.tspackages/provider/src/factory.tspackages/provider/src/index.tspackages/provider/src/models.tspackages/provider/src/openai-compatible.tspackages/provider/src/openai.test.tspackages/provider/src/openai.ts
Contributor
Author
|
Thanks @coderabbitai — addressed all five findings (pushed in
Gate green: 257 pass, 0 fail. |
|
Tip For best results, initiate chat on the files or code changes.
257/257 green is a solid gate. 🎉 (ᵔ◡ᵔ)🐇✨ If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Follow-up to #15. Adds DeepSeek, Aliyun Bailian (Qwen), Moonshot (Kimi), and Zhipu GLM — all of which expose OpenAI-compatible Chat Completions APIs, so they reuse the existing
OpenAIProvider(pointed at a different base URL + key) rather than new SDK adapters. The structure is a small registry so the next OpenAI-compatible vendor is a one-line entry.Approach
OpenAIProvidermade reusable — addsname(so traces/cost showdeepseek/bailian, notopenai),maxTokensField(compatible gateways usemax_tokens, notmax_completion_tokens), and a stop-reason hardening: if a tool_call is present,stop_reasonistool_useregardless offinish_reason(some gateways reportstopwhile still returningtool_calls— previously that dropped the calls).OPENAI_COMPATIBLEregistry (packages/provider/src/openai-compatible.ts) — one entry per vendor:baseURL(+*_BASE_URLenv override),apiKeyEnv[], modelprefixes,maxTokensField. OpenAI itself is the canonical entry.resolveModel(modelId) → { kind, model }drives routing: prefix inference (deepseek-*→ DeepSeek,qwen*→ Bailian,kimi*/moonshot*→ Moonshot,glm-*→ Zhipu), plus an explicitvendor/modeloverride (e.g.bailian/deepseek-r1) that forces the backend and strips the prefix so the bare id hits the API and pricing. The provider is resolved from the raw id (so the override's forced backend wins) while the Worker runs the clean id.apps/api/src/runner.ts,packages/cli/src/main.ts) useresolveModelfor the backend + clean model id, gating onhasCredentials(kind).Adding the next vendor
One entry in
OPENAI_COMPATIBLE(label, baseURL, apiKeyEnv, prefixes, maxTokensField) + a pricing row. That's it.Pricing (verified against official sources)
Verified each rate against the provider's official pricing page (USD per 1M tokens, input is cache-miss). This also surfaced that several example ids picked earlier were deprecated/EOL as of June 2026, so those were refreshed to current models:
deepseek-chat/deepseek-reasoner(alias →deepseek-v4-flash; deprecate 2026-07-24)deepseek-v4-proqwen-plusqwen-maxqwen-turbokimi-k2.5kimi-k2.6glm-4.7glm-4.7-flashxRetired ids removed/replaced:
moonshot-v1-8k/moonshot-v1-32kand the legacykimi-k2-0905-preview(EOL), andglm-4/glm-4-plus(superseded by the GLM-4.7 family). Rates change frequently and the table stays configurable.Tests
Hermetic:
OpenAIProvidername/max_tokens/stop-reason + no-OPENAI_API_KEY-fallback cases; factory prefix table + thevendor/modeloverride (incl. prototype-key and empty-override guards) + new-kind credential checks; pricing cases. Full gate green: 257 pass, 0 fail.Notes
deepseek-reasoner/R1) can't drive the harness's tool loop;deepseek-chat,qwen-*,kimi-*,glm-4.7are the harness-friendly ones (documented).DASHSCOPE_BASE_URLswitches to international.Post-review updates
baseURLnever inheritsOPENAI_API_KEY(would otherwise send the OpenAI key to a third party) — fails closed instead.resolveModelhardened against inherited object keys (toString/…) and emptyvendor/overrides.🤖 Generated with Claude Code
Summary by CodeRabbit
vendor/modeloverride behavior).