fix(model): handle minimax m3#1771
Merged
Merged
Conversation
Contributor
|
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 (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughFixes MiniMax model ID matching by normalizing provider DB candidate IDs (stripping ChangesMiniMax model matching and adaptive thinking
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@docs/issues/minimax-model-matching/spec.md`:
- Around line 5-7: The compound modifier in the documentation is missing proper
hyphenation for readability. Change "provider DB derived configuration" to
"provider DB-derived configuration" by adding a hyphen between "DB" and
"derived" to properly format the compound modifier that describes the
configuration type.
In `@src/main/presenter/configPresenter/modelConfig.ts`:
- Around line 195-201: The MiniMax-M3 context floor computed at the
isMiniMaxM3Model check is being silently overridden by stale provider cache
values during the merge at the provider source preference logic (line 534).
Ensure that when merging provider-specific configurations, the M3 context floor
is not bypassed by cached values. Either prevent the storedConfig.contextLength
preference from applying to M3 models, or reapply the M3 floor constraint after
the provider cache merge in the applyProviderSpecificPolicies method to
guarantee the floor remains intact regardless of cached state.
🪄 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: e78a84d2-df21-44c5-95b5-0a59785d22b0
📒 Files selected for processing (7)
docs/issues/minimax-model-matching/plan.mddocs/issues/minimax-model-matching/spec.mddocs/issues/minimax-model-matching/tasks.mdsrc/main/presenter/configPresenter/modelConfig.tssrc/main/presenter/llmProviderPresenter/aiSdk/providerOptionsMapper.tstest/main/presenter/llmProviderPresenter/aiSdkProviderOptionsMapper.test.tstest/main/presenter/providerDbModelConfig.test.ts
Comment on lines
+195
to
+201
| const contextLimit = isMiniMaxM3Model(providerId, model.id) | ||
| ? Math.max(model.limit?.context ?? 0, MINIMAX_M3_CONTEXT_LENGTH) | ||
| : model.limit?.context | ||
|
|
||
| return this.applyProviderSpecificPolicies(providerId, model.id, { | ||
| maxTokens: resolveDerivedModelMaxTokens(model.limit?.output), | ||
| contextLength: resolveModelContextLength(model.limit?.context), | ||
| contextLength: resolveModelContextLength(contextLimit), |
Contributor
There was a problem hiding this comment.
MiniMax-M3 context floor can be silently bypassed by provider cache merge.
You compute the M3 floor here, but Line 534 later prefers storedConfig.contextLength for source: 'provider' entries. A stale provider cache value (e.g., 512000) will override the new 1_000_000 minimum and break the intended fallback behavior.
Proposed fix
- contextLength: storedConfig.contextLength ?? finalConfig.contextLength,
+ contextLength:
+ isMiniMaxM3Model(providerId, modelId)
+ ? Math.max(
+ storedConfig.contextLength ?? 0,
+ finalConfig.contextLength ?? MINIMAX_M3_CONTEXT_LENGTH
+ )
+ : storedConfig.contextLength ?? finalConfig.contextLength,🤖 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/main/presenter/configPresenter/modelConfig.ts` around lines 195 - 201,
The MiniMax-M3 context floor computed at the isMiniMaxM3Model check is being
silently overridden by stale provider cache values during the merge at the
provider source preference logic (line 534). Ensure that when merging
provider-specific configurations, the M3 context floor is not bypassed by cached
values. Either prevent the storedConfig.contextLength preference from applying
to M3 models, or reapply the M3 floor constraint after the provider cache merge
in the applyProviderSpecificPolicies method to guarantee the floor remains
intact regardless of cached state.
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.
Summary by CodeRabbit