fix: update Deepseek configurations - #1083
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDeepSeek V4 Flash and Pro metadata now use revised reasoning options and pricing. Retired model IDs were removed. Model-specific thinking behavior and tests now target V4 models. The default temperature changed from ChangesDeepSeek V4 support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Request as DeepSeek request builder
participant Normalizer as reasoning-effort normalizer
participant API as DeepSeek V4 API
Request->>Normalizer: pass model ID and reasoning effort
Normalizer->>Normalizer: apply Flash or Pro mapping
Normalizer->>API: send normalized thinking parameters
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
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 `@packages/types/src/providers/deepseek.ts`:
- Line 27: Update DeepSeekHandler.getModel() to map deepseek-chat and
deepseek-reasoner to deepseek-v4-flash before model lookup and before
constructing requestOptions.model, while preserving the legacy thinking behavior
associated with each retired ID.
- Line 17: Update the DeepSeek V4 model metadata in DeepSeekHandler so
non-thinking mode remains discoverable: add "disable" to both
supportsReasoningEffort arrays, or consistently introduce and use a separate
supportsReasoningBinary capability while preserving the existing thinking.type:
"disabled" behavior.
🪄 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 Plus
Run ID: f8d15a12-9b4d-4fe0-96da-18c097797209
📒 Files selected for processing (1)
packages/types/src/providers/deepseek.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/api/providers/__tests__/deepseek.spec.ts (2)
248-248: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winReplace removed legacy tests with migration tests.
These deletions remove coverage for
deepseek-reasonerthinking behavior anddeepseek-chatnon-thinking behavior. New V4 tests do not verify existing profiles that still contain retired IDs. Add tests for the migrated request model and itsthinkingvalue for both aliases. (raw.githubusercontent.com)Also applies to: 617-620
🤖 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/api/providers/__tests__/deepseek.spec.ts` at line 248, Add migration coverage in the DeepSeek provider tests for both aliases: verify the migrated request model and its expected thinking value for deepseek-reasoner and deepseek-chat, including the existing profiles that reference retired IDs. Replace the removed legacy-test coverage without changing unrelated default-model behavior.Source: Coding guidelines
229-229: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the complete reasoning contract.
toContain("max")does not prove that retired options are absent. The removed test also leaves no coverage for Flashxhigh → high, Prolow → high, or Proxhigh → max. Assert the exact option arrays and add request-construction cases for these mappings anddisable. This follows the package-local unit-test guideline. (raw.githubusercontent.com)Also applies to: 583-583
🤖 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/api/providers/__tests__/deepseek.spec.ts` at line 229, Update the reasoning-effort assertions in the DeepSeek provider tests to compare complete option arrays, ensuring retired options are absent. Extend request-construction coverage for Flash xhigh→high, Pro low→high, Pro xhigh→max, and disable mappings, using the existing test helpers and model symbols around the affected cases.Source: Coding guidelines
🤖 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/api/providers/deepseek.ts`:
- Line 40: Update the reasoning-effort normalization flow around
supportsDeepSeekThinkingToggle and the downstream normalizer so it receives
modelId or equivalent model metadata. Apply model-specific mappings: Flash
preserves low and converts xhigh to high; Pro converts low to high and xhigh to
max. Ensure the output type continues to include low.
---
Nitpick comments:
In `@src/api/providers/__tests__/deepseek.spec.ts`:
- Line 248: Add migration coverage in the DeepSeek provider tests for both
aliases: verify the migrated request model and its expected thinking value for
deepseek-reasoner and deepseek-chat, including the existing profiles that
reference retired IDs. Replace the removed legacy-test coverage without changing
unrelated default-model behavior.
- Line 229: Update the reasoning-effort assertions in the DeepSeek provider
tests to compare complete option arrays, ensuring retired options are absent.
Extend request-construction coverage for Flash xhigh→high, Pro low→high, Pro
xhigh→max, and disable mappings, using the existing test helpers and model
symbols around the affected cases.
🪄 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 Plus
Run ID: 706a6d21-9119-4137-aee4-44f35d267201
📒 Files selected for processing (5)
packages/types/src/providers/deepseek.tssrc/api/providers/__tests__/deepseek.spec.tssrc/api/providers/deepseek.tssrc/core/config/__tests__/importExport.spec.tssrc/core/webview/__tests__/webviewMessageHandler.routerModels.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/types/src/providers/deepseek.ts
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
All discussions started by the bot are resolved. This is the first time I submit a PR to a project set up with an AI pipeline. I am truly impressed! Thank you for your great work of maintaining Zoo Code! |
github-actions
Bot
added
the
awaiting-review
|
|
||
| // https://api-docs.deepseek.com/quick_start/parameter_settings | ||
| export const DEEP_SEEK_DEFAULT_TEMPERATURE = 0.3 | ||
| export const DEEP_SEEK_DEFAULT_TEMPERATURE = 0.0 |
There was a problem hiding this comment.
I'm not sure about setting this to 0 completely. I know their docs say 0 for coding, but I think you still need a bit of variance there for 'creativity', 0.3 felt right to me.
There was a problem hiding this comment.
I can remove this change if you insist. But according to https://api-docs.deepseek.com/guides/thinking_mode:
Thinking mode does not support the temperature, top_p, presence_penalty, or frequency_penalty parameters.
So it's only going to affect the case when the reasoning effor is set to "disable" anyway. Besides, this is just the default value. The users can always adjust the actual temperature in the extension.
github-actions
Bot
added
awaiting-author
|
Right now it maps every value except Zoo Code supports a broad shared set of reasoning values, while each DeepSeek model accepts a smaller set. The normalizer should preserve supported values and only translate old or unsupported ones:
Suggested change: pass |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/api/providers/deepseek.ts (1)
36-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemoving the
deepseek-reasoneralias check makes thethinkingternary's fallback branch dead code.After this change,
isDeepSeekThinkingEnabledreturns exactlysupportsDeepSeekThinkingToggle(modelId)once the explicit-disable checks are cleared. This meansisThinkingModelcan only betruewhensupportsDeepSeekThinkingToggle(modelId)is alsotrue.Downstream, at Lines 133-137, the
thinkingcomputation is:const thinking = supportsDeepSeekThinkingToggle(modelId) ? ({ type: isThinkingModel ? "enabled" : "disabled" } as const) : isThinkingModel ? ({ type: "enabled" } as const) : undefinedThe
: isThinkingModel ? ({ type: "enabled" } as const) : undefinedbranch can never select{ type: "enabled" }, because reaching it requiressupportsDeepSeekThinkingToggle(modelId)to befalse, which forcesisThinkingModelto also befalse. This branch is now unreachable dead code. Simplify it to reduce confusion for future maintainers.♻️ Proposed simplification
- const thinking = supportsDeepSeekThinkingToggle(modelId) - ? ({ type: isThinkingModel ? "enabled" : "disabled" } as const) - : isThinkingModel - ? ({ type: "enabled" } as const) - : undefined + const thinking = supportsDeepSeekThinkingToggle(modelId) + ? ({ type: isThinkingModel ? "enabled" : "disabled" } as const) + : undefined🤖 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/api/providers/deepseek.ts` around lines 36 - 42, In the thinking computation inside the DeepSeek provider, simplify the fallback branch because isThinkingModel cannot be true when supportsDeepSeekThinkingToggle(modelId) is false. Preserve the enabled/disabled object selection when the toggle is supported, and return undefined directly for the unsupported-toggle case.
🤖 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/api/providers/deepseek.ts`:
- Around line 44-84: Update normalizeDeepSeekReasoningEffort to handle
reasoningEffort === "medium" in both model-specific switches: return "high" for
deepseek-v4-flash and deepseek-v4-pro, while preserving all existing mappings
and the undefined fallback for unsupported values.
---
Nitpick comments:
In `@src/api/providers/deepseek.ts`:
- Around line 36-42: In the thinking computation inside the DeepSeek provider,
simplify the fallback branch because isThinkingModel cannot be true when
supportsDeepSeekThinkingToggle(modelId) is false. Preserve the enabled/disabled
object selection when the toggle is supported, and return undefined directly for
the unsupported-toggle case.
🪄 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 Plus
Run ID: 958909da-1d1e-4aa6-9df1-1c1f61eaba4f
📒 Files selected for processing (5)
packages/types/src/providers/deepseek.tssrc/api/providers/__tests__/deepseek.spec.tssrc/api/providers/deepseek.tssrc/core/config/__tests__/importExport.spec.tssrc/core/webview/__tests__/webviewMessageHandler.routerModels.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/core/config/tests/importExport.spec.ts
- src/core/webview/tests/webviewMessageHandler.routerModels.spec.ts
- src/api/providers/tests/deepseek.spec.ts
- packages/types/src/providers/deepseek.ts
I just added the fix in a new commit and unit testing in another. |
taltas
left a comment
taltas
left a comment
There was a problem hiding this comment.
Thanks for addressing the feedback. I'm approving the PR.
2 participants
Related GitHub Issue
Closes: #1082
Description
Fixed the misalignments between the current Zoo Code Deepseek configurations and the official documentation. Check @1082 for more details.
Test Procedure
Built the extension locally and verified that it's working on my system.
Pre-Submission Checklist
Visual Snapshot (UI changes only): If a user would notice this change at a glance (layout, theme tokens, brand elements, empty/error states), I've added or updated a*.visual.tsxsnapshot inwebview-ui/. Seewebview-ui/AGENTS.md→ "When a UI change needs a snapshot".Visual Snapshots
N/A
Videos (interaction / animation only)
N/A
Documentation Updates
N/A
Additional Notes
N/A
Get in Touch
I don't use Discord. Please comment in this PR or the linked issue. I signed up for email notifications and will respond promptly.
Summary by CodeRabbit