refactor(providers): migrate deepseek to native OpenAIChatCompletionsProvider base#1057
refactor(providers): migrate deepseek to native OpenAIChatCompletionsProvider base#1057murdore wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughRefactors DeepSeekProvider to extend OpenAIChatCompletionsProvider, delegates initialization to the base class, removes in-file streaming/execution orchestration, and retains small DeepSeek hooks for fallback models and response_format compatibility. ChangesDeepSeek to OpenAI-compatibility provider migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/lib/providers/deepseek.tsParsing error: Unable to parse the specified 'tsconfig' file. Ensure it's correct and has valid syntax. error TS5012: Cannot read file '/.svelte-kit/tsconfig.json': ENOENT: no such file or directory, open '/.svelte-kit/tsconfig.json'. 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 |
cf9b18d to
62d25bd
Compare
877f888 to
a1051a5
Compare
a1051a5 to
12b5690
Compare
62d25bd to
5928e1e
Compare
12b5690 to
b2ef907
Compare
b2ef907 to
d68ad57
Compare
✅ Single Commit Policy - COMPLIANTStatus: Policy requirements met • 1 commit • Valid format • Ready for merge 📊 View validation details📝 Commit Details
✅ Validation Results
🤖 Automated validation by NeuroLink Single Commit Enforcement |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
There was a problem hiding this comment.
Pull request overview
This PR migrates the DeepSeek provider off the Vercel AI SDK and onto the native OpenAIChatCompletionsProvider HTTP+SSE base, aiming to preserve existing behavior while centralizing request/stream/tool-loop orchestration in the shared base.
Changes:
- Refactors
DeepSeekProviderto extendOpenAIChatCompletionsProviderand wires constructor config (apiKey/baseURL) into the base. - Keeps DeepSeek-specific behavior via hooks:
response_formatdowngrade (json_schema→json_object) and “thinking mode” enablement logic.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| protected adjustBuildBodyOptions( | ||
| modelId: string, | ||
| opts: OpenAICompatBuildBodyArgs["options"], | ||
| ): OpenAICompatBuildBodyArgs["options"] { | ||
| const isReasoner = modelId === DeepSeekModels.DEEPSEEK_REASONER; | ||
| const thinkingConfig = (opts as Record<string, unknown>).thinkingConfig as | ||
| | { enabled?: boolean } | ||
| | undefined; | ||
| if (!isReasoner && thinkingConfig?.enabled === true) { | ||
| return { | ||
| ...opts, | ||
| // Forwarded to the request body as a top-level `thinking` field. | ||
| thinking: { type: "enabled" }, | ||
| } as OpenAICompatBuildBodyArgs["options"]; | ||
| } | ||
| return opts; |
There was a problem hiding this comment.
Removed. Confirmed the thinking branch was a no-op both pre- and post-migration: buildBody() drops unknown option keys (so thinking never reached the wire), and pre-migration the flag was set under providerOptions.openai while @ai-sdk/openai-compatible reads providerOptions[providerOptionsName] = "deepseek" — so it was never forwarded there either. Dead code removed; proper DeepSeek reasoning support (correctly-keyed thinking passthrough + reasoning_content) is tracked as a follow-up.
| * (The json_object word-injection DeepSeek's API requires is handled | ||
| * generically by the base client's `ensureJsonWord`, alongside the | ||
| * `adjustResponseFormat` json_schema→json_object downgrade below.) |
There was a problem hiding this comment.
Fixed — docstring corrected from ensureJsonWord to ensureJsonWordInBody.
d68ad57 to
b63a297
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
b63a297 to
43db35c
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
|
@coderabbitai are you reviewing or not? |
|
(´・ω・`) 🐇 ✅ Actions performedFull review triggered. |
| protected adjustBuildBodyOptions( | ||
| modelId: string, | ||
| opts: OpenAICompatBuildBodyArgs["options"], | ||
| ): OpenAICompatBuildBodyArgs["options"] { | ||
| const isReasoner = modelId === DeepSeekModels.DEEPSEEK_REASONER; | ||
| const thinkingConfig = (opts as Record<string, unknown>).thinkingConfig as | ||
| | { enabled?: boolean } | ||
| | undefined; | ||
| if (!isReasoner && thinkingConfig?.enabled === true) { | ||
| return { | ||
| ...opts, | ||
| // Forwarded to the request body as a top-level `thinking` field. | ||
| thinking: { type: "enabled" }, | ||
| } as OpenAICompatBuildBodyArgs["options"]; |
There was a problem hiding this comment.
Same as above — the thinking field never reached the wire (buildBody drops unknown keys; and pre-migration the providerOptions.openai key wasn't read by the name:"deepseek" openai-compatible client). Removed as dead code; reasoning support tracked as a follow-up.
| /** | ||
| * DeepSeek's /chat/completions rejects `response_format: { type: | ||
| * "json_schema" }` outright ("This response_format type is unavailable | ||
| * now"). The `@ai-sdk/openai-compatible` provider this migration replaced | ||
| * ran with `supportsStructuredOutputs: false`, which downgraded structured- | ||
| * output requests to `{ type: "json_object" }`. Replicate that downgrade so | ||
| * `generate({ schema })` keeps working. (DeepSeek's json_object mode also | ||
| * requires the word "json" somewhere in the messages; the base client's | ||
| * `ensureJsonWord` injects a minimal instruction when the prompt lacks it.) | ||
| */ | ||
| protected adjustResponseFormat( | ||
| rf: OpenAICompatResponseFormat | undefined, | ||
| _modelId: string, | ||
| ): OpenAICompatResponseFormat | undefined { | ||
| if (rf?.type === "json_schema") { | ||
| return { type: "json_object" }; | ||
| } | ||
| return rf; | ||
| } |
There was a problem hiding this comment.
Verified live — the json_schema → json_object downgrade does NOT drop schema guidance. generate({ schema }) against deepseek-chat returns schema-conforming output ({"name":"France","capital":"Paris","population":67391582}): NeuroLink injects the schema into the prompt independently of response_format, so downgrading to json_object (required because DeepSeek rejects json_schema) keeps structured output working. No change needed.
| * (The json_object word-injection DeepSeek's API requires is handled | ||
| * generically by the base client's `ensureJsonWord`, alongside the | ||
| * `adjustResponseFormat` json_schema→json_object downgrade below.) |
There was a problem hiding this comment.
Fixed — docstring reference corrected to ensureJsonWordInBody.
…Provider base Replace the @ai-sdk/openai-compatible implementation with the native OpenAIChatCompletionsProvider base. DeepSeek speaks OpenAI-compatible chat completions at api.deepseek.com; the subclass declares config plus one quirk: adjustResponseFormat downgrades the json_schema response_format (which DeepSeek rejects) to json_object, matching the prior supportsStructuredOutputs:false behaviour. Structured output was verified live to still conform to the requested schema. Review fixes: - baseURL precedence now trims blank/whitespace overrides so an empty credentials.baseURL or DEEPSEEK_BASE_URL= cannot override the default with "" (mirrors the apiKey guard above). - Correct docstring references: ensureJsonWord -> ensureJsonWordInBody. - Remove the non-functional thinking-mode branch: the thinkingConfig signal is never plumbed into the native v3 delegate's build options on the non-streaming path, and buildBody drops unknown keys on both paths, so `thinking` never reached the wire. Restoring DeepSeek reasoning (thinking passthrough + reasoning_content surfacing) needs base-client plumbing and is tracked as a follow-up; the dead code is removed so the provider does not imply support it lacks.
43db35c to
f8eff7e
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/providers/deepseek.ts (1)
82-96: ⚡ Quick winPrefer the
AIProviderName.DEEPSEEKenum over the"deepseek" as AIProviderNamecast.The string cast bypasses the enum's type guarantee at both call sites (Line 82 and Line 96); if the enum value ever drifts, this won't be caught at compile time. Using the enum value requires a value import instead of the current type-only import on Line 1.
♻️ Use the enum value
-import type { AIProviderName } from "../constants/enums.js"; -import { DeepSeekModels } from "../constants/enums.js"; +import { AIProviderName, DeepSeekModels } from "../constants/enums.js";- super("deepseek" as AIProviderName, modelName, sdk, { baseURL, apiKey }); + super(AIProviderName.DEEPSEEK, modelName, sdk, { baseURL, apiKey });protected getProviderName(): AIProviderName { - return "deepseek" as AIProviderName; + return AIProviderName.DEEPSEEK; }🤖 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/lib/providers/deepseek.ts` around lines 82 - 96, Replace the string-cast usages of the provider name with the enum value to preserve type guarantees: change the call to super("deepseek" as AIProviderName, modelName, sdk, ...) and the return in getProviderName() from "deepseek" as AIProviderName to AIProviderName.DEEPSEEK; also update the import at the top to import the AIProviderName value (not only a type-only import) so AIProviderName.DEEPSEEK is available at runtime. Ensure both the constructor call (super(...)) and the method getProviderName() use AIProviderName.DEEPSEEK.
🤖 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/lib/providers/deepseek.ts`:
- Around line 49-54: Update the release notes/docs to explicitly state that
DeepSeek non-reasoner chat models do not accept the `thinking` param in the
OpenAI-compatible client path and that `thinkingConfig` is only translated for
Anthropic and Google Gemini 3; document that "high"/reasoning use the
`deepseek-reasoner` model (not `deepseek-chat`) and that `deepseek-chat` returns
empty reasoning for `thinking.minimal` (match tests E1/E2). Mention the relevant
symbols: `thinkingConfig`, `thinking.high`, `thinking.minimal`,
`deepseek-reasoner`, and `deepseek-chat`, and note that plumbing for
`reasoning_content` is a base-client follow-up.
---
Nitpick comments:
In `@src/lib/providers/deepseek.ts`:
- Around line 82-96: Replace the string-cast usages of the provider name with
the enum value to preserve type guarantees: change the call to super("deepseek"
as AIProviderName, modelName, sdk, ...) and the return in getProviderName() from
"deepseek" as AIProviderName to AIProviderName.DEEPSEEK; also update the import
at the top to import the AIProviderName value (not only a type-only import) so
AIProviderName.DEEPSEEK is available at runtime. Ensure both the constructor
call (super(...)) and the method getProviderName() use AIProviderName.DEEPSEEK.
🪄 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: CHILL
Plan: Pro
Run ID: 29d5ffe0-d7f4-4f64-b2fb-e13b4f2e0444
📒 Files selected for processing (1)
src/lib/providers/deepseek.ts
|
Addressed review feedback (
|
|
Clarification on the removed thinking branch — confirmed it was a no-op before this migration too, so removing it is not a behavior change: The pre-migration provider set `providerOptions: { openai: { thinking: { type: "enabled" } } }`, but it was built with `createOpenAICompatible({ name: "deepseek" })`. `@ai-sdk/openai-compatible` reads provider options under `providerOptions[providerOptionsName]` where `providerOptionsName = config.provider.split(".")[0]` = `"deepseek"` — so the `openai`-keyed options were never forwarded to the wire. Thinking was dead on the old path as well; this is pure dead-code cleanup with no backward-compatibility impact. Proper DeepSeek reasoning support (thinking passthrough keyed correctly + `reasoning_content` surfacing) remains a tracked follow-up. |
What
Migrates the DeepSeek provider off the Vercel AI SDK to the native
OpenAIChatCompletionsProviderbase. Final batch of the AI-SDK removal (after the 10 thin-subclass migrations #1045-#1054 and the base hooks #1055).Behavior-preserving
Test plan
Summary by CodeRabbit