fix: modify deepseek's context to 1m#431
fix: modify deepseek's context to 1m#431cynicalight wants to merge 3 commits intoclaude-code-best:mainfrom
Conversation
📝 WalkthroughWalkthroughThe PR adds a special-case model context window resolution for deepseek-v4. When a model name includes ChangesDeepseek v4 Context Resolution
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 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 unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
🧹 Nitpick comments (2)
src/utils/context.ts (2)
103-105: ⚡ Quick winAdd a documentation comment.
Consider adding a brief comment explaining why deepseek-v4 requires special handling and referencing the context window specification:
+ // DeepSeek v4 models support 1M context window if (model.toLowerCase().includes('deepseek-v4')) { return 1_000_000 }This helps future maintainers understand the intent and makes it easier to update when model specifications change.
🤖 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/utils/context.ts` around lines 103 - 105, Add a brief documentation comment immediately above the if statement that checks model.toLowerCase().includes('deepseek-v4') explaining that deepseek-v4 uses an extended/context window of 1,000,000 tokens (hence returning 1_000_000 here), include a short rationale or link/reference to the model's context window specification and note that this branch is special-cased so it can be updated if the model spec changes; reference the exact symbols `model.toLowerCase().includes('deepseek-v4')` and the returned literal `1_000_000` so maintainers know where to update.
103-105: ⚡ Quick winAdd a comment explaining deepseek-v4 1M context support.
The substring match
model.toLowerCase().includes('deepseek-v4')is consistent with how other third-party models are checked elsewhere in this file (e.g.,deepseek-v4-proin effort.ts). However, the check could benefit from a comment explaining why deepseek-v4 models support 1M context window, similar to the annotation at line 43 (@[MODEL LAUNCH]).Consider adding:
// DeepSeek V4 models support 1M context window if (model.toLowerCase().includes('deepseek-v4')) { return 1_000_000 }🤖 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/utils/context.ts` around lines 103 - 105, Add a brief explanatory comment above the existing model.toLowerCase().includes('deepseek-v4') check clarifying that DeepSeek V4 models support a 1,000,000-token context window (matching the style of the existing @[MODEL LAUNCH] annotation), then leave the conditional that returns 1_000_000 unchanged so the function continues to treat any model string containing 'deepseek-v4' as 1M context-capable.
🤖 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.
Nitpick comments:
In `@src/utils/context.ts`:
- Around line 103-105: Add a brief documentation comment immediately above the
if statement that checks model.toLowerCase().includes('deepseek-v4') explaining
that deepseek-v4 uses an extended/context window of 1,000,000 tokens (hence
returning 1_000_000 here), include a short rationale or link/reference to the
model's context window specification and note that this branch is special-cased
so it can be updated if the model spec changes; reference the exact symbols
`model.toLowerCase().includes('deepseek-v4')` and the returned literal
`1_000_000` so maintainers know where to update.
- Around line 103-105: Add a brief explanatory comment above the existing
model.toLowerCase().includes('deepseek-v4') check clarifying that DeepSeek V4
models support a 1,000,000-token context window (matching the style of the
existing @[MODEL LAUNCH] annotation), then leave the conditional that returns
1_000_000 unchanged so the function continues to treat any model string
containing 'deepseek-v4' as 1M context-capable.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/utils/context.ts (1)
103-110: ⚖️ Poor tradeoffPrefer registering third-party model capabilities in
modelCapabilitiesrather than hardcoding strings here.The function already handles third-party models generically at lines 79–88 via
getModelCapability(model). Adding raw substring checks insidegetContextWindowForModelcreates an inconsistent dual-path: if a model is registered inmodelCapabilitieswithmax_input_tokens >= 100_000, the new block (placed after that check) is dead code for it; if it isn't registered, the new block fires — but only after bypassing the proper capability-lookup mechanism.Adding deepseek-v4 to the
modelCapabilitiesregistry withmax_input_tokens: 1_000_000would be handled automatically by the existing logic and would keep context metadata centralized.🤖 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/utils/context.ts` around lines 103 - 110, The substring checks for third-party models inside getContextWindowForModel (the lower.includes('deepseek-v4') / 'gpt-5.4' / 'gpt-5.5' block) should be removed and the capability for deepseek-v4 (and any other third-party model) should be added to the centralized modelCapabilities registry so getModelCapability(model) returns the correct max_input_tokens (e.g., set deepseek-v4.max_input_tokens to 1_000_000); update modelCapabilities instead of hardcoding in getContextWindowForModel and ensure getContextWindowForModel continues to call getModelCapability to derive the context window.
🤖 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/utils/context.ts`:
- Around line 103-110: The new branch that returns 1_000_000 for models matching
'deepseek-v4', 'gpt-5.4' or 'gpt-5.5' bypasses the HIPAA toggle; update that
block to first call the existing guard (is1mContextDisabled()) or reuse the
helper flow (e.g., has1mContext() / modelSupports1M() /
getSonnet1mExpTreatmentEnabled()) and only return 1_000_000 when the guard
permits it, otherwise fall through to the normal behavior; locate the block
around the model.toLowerCase() check and add the same disable-check logic used
by other 1M-returning paths.
- Around line 106-108: The current substring checks lower.includes('gpt-5.4')
and lower.includes('gpt-5.5') over-match smaller variants and use an inaccurate
context size; replace those includes checks with a tighter match (e.g., a regex
or exact token match that accepts only the full 5.4/5.4-pro and 5.5 model names
and explicitly excludes -mini/-nano) so you don't count low-cost variants, and
update the reported context window from 1_000_000 to the correct 1_050_000 value
used for GPT-5.4/5.5; locate the condition containing lower.includes('gpt-5.4')
and lower.includes('gpt-5.5') and adjust the matching logic and constant
accordingly.
---
Nitpick comments:
In `@src/utils/context.ts`:
- Around line 103-110: The substring checks for third-party models inside
getContextWindowForModel (the lower.includes('deepseek-v4') / 'gpt-5.4' /
'gpt-5.5' block) should be removed and the capability for deepseek-v4 (and any
other third-party model) should be added to the centralized modelCapabilities
registry so getModelCapability(model) returns the correct max_input_tokens
(e.g., set deepseek-v4.max_input_tokens to 1_000_000); update modelCapabilities
instead of hardcoding in getContextWindowForModel and ensure
getContextWindowForModel continues to call getModelCapability to derive the
context window.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| const lower = model.toLowerCase() | ||
| if ( | ||
| lower.includes('deepseek-v4') || | ||
| lower.includes('gpt-5.4') || | ||
| lower.includes('gpt-5.5') | ||
| ) { | ||
| return 1_000_000 | ||
| } |
There was a problem hiding this comment.
Missing is1mContextDisabled() guard bypasses HIPAA compliance control.
Every other code path that returns 1_000_000 first checks is1mContextDisabled() (directly or via has1mContext() / modelSupports1M() / getSonnet1mExpTreatmentEnabled()). The new block skips this check entirely, so setting CLAUDE_CODE_DISABLE_1M_CONTEXT=1 — documented as the mechanism for C4E admins to enforce HIPAA compliance — has no effect on deepseek-v4 or the gpt-5.x names added here.
🛡️ Proposed fix
+ if (!is1mContextDisabled()) {
const lower = model.toLowerCase()
if (
lower.includes('deepseek-v4') ||
lower.includes('gpt-5.4') ||
lower.includes('gpt-5.5')
) {
return 1_000_000
}
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const lower = model.toLowerCase() | |
| if ( | |
| lower.includes('deepseek-v4') || | |
| lower.includes('gpt-5.4') || | |
| lower.includes('gpt-5.5') | |
| ) { | |
| return 1_000_000 | |
| } | |
| if (!is1mContextDisabled()) { | |
| const lower = model.toLowerCase() | |
| if ( | |
| lower.includes('deepseek-v4') || | |
| lower.includes('gpt-5.4') || | |
| lower.includes('gpt-5.5') | |
| ) { | |
| return 1_000_000 | |
| } | |
| } |
🤖 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/utils/context.ts` around lines 103 - 110, The new branch that returns
1_000_000 for models matching 'deepseek-v4', 'gpt-5.4' or 'gpt-5.5' bypasses the
HIPAA toggle; update that block to first call the existing guard
(is1mContextDisabled()) or reuse the helper flow (e.g., has1mContext() /
modelSupports1M() / getSonnet1mExpTreatmentEnabled()) and only return 1_000_000
when the guard permits it, otherwise fall through to the normal behavior; locate
the block around the model.toLowerCase() check and add the same disable-check
logic used by other 1M-returning paths.
| lower.includes('gpt-5.4') || | ||
| lower.includes('gpt-5.5') | ||
| ) { |
There was a problem hiding this comment.
gpt-5.4 substring over-matches smaller variants, and the hardcoded value is slightly inaccurate.
lower.includes('gpt-5.4') will also match gpt-5.4-mini and gpt-5.4-nano, which are lower-cost variants that do not carry a 1M context window. Additionally, GPT-5.4 and GPT-5.4 pro have a 1.05M context window, and GPT-5.5 also has a 1,050,000 context window — so the hardcoded 1_000_000 slightly under-reports the true limit (though it is conservative and safe for auto-compact purposes).
🐛 Proposed fix — tighten the match and use the accurate context size
- const lower = model.toLowerCase()
- if (
- lower.includes('deepseek-v4') ||
- lower.includes('gpt-5.4') ||
- lower.includes('gpt-5.5')
- ) {
- return 1_000_000
- }
+ const lower = model.toLowerCase()
+ // deepseek-v4: documented 1M context
+ if (lower.includes('deepseek-v4')) {
+ return 1_000_000
+ }
+ // gpt-5.4 / gpt-5.5: 1.05M context window per OpenAI docs.
+ // Exclude mini/nano variants which have smaller context windows.
+ if (
+ (lower.includes('gpt-5.4') && !lower.includes('gpt-5.4-mini') && !lower.includes('gpt-5.4-nano')) ||
+ lower.includes('gpt-5.5')
+ ) {
+ return 1_050_000
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| lower.includes('gpt-5.4') || | |
| lower.includes('gpt-5.5') | |
| ) { | |
| const lower = model.toLowerCase() | |
| // deepseek-v4: documented 1M context | |
| if (lower.includes('deepseek-v4')) { | |
| return 1_000_000 | |
| } | |
| // gpt-5.4 / gpt-5.5: 1.05M context window per OpenAI docs. | |
| // Exclude mini/nano variants which have smaller context windows. | |
| if ( | |
| (lower.includes('gpt-5.4') && !lower.includes('gpt-5.4-mini') && !lower.includes('gpt-5.4-nano')) || | |
| lower.includes('gpt-5.5') | |
| ) { | |
| return 1_050_000 | |
| } |
🤖 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/utils/context.ts` around lines 106 - 108, The current substring checks
lower.includes('gpt-5.4') and lower.includes('gpt-5.5') over-match smaller
variants and use an inaccurate context size; replace those includes checks with
a tighter match (e.g., a regex or exact token match that accepts only the full
5.4/5.4-pro and 5.5 model names and explicitly excludes -mini/-nano) so you
don't count low-cost variants, and update the reported context window from
1_000_000 to the correct 1_050_000 value used for GPT-5.4/5.5; locate the
condition containing lower.includes('gpt-5.4') and lower.includes('gpt-5.5') and
adjust the matching logic and constant accordingly.
|
额, 你只改了显示, 没有改其他的判断 按照 cc 的逻辑, 你得给模型名称加上 [1m] 就会判断成 1m 上下文 @cynicalight |
我看到了那个加1m的逻辑了，感觉有点怪，不应该在名称里指定吧感觉，他自己的模型的一些context参数都是hardcode的 |
2 participants
小修改，增加了对 deepseek-v4 的特判，context 正确显示为 1m，原本会 fallback 到 200k
Need help on this PR? Tag
@codesmithwith what you need.Summary by CodeRabbit