Skip to content

feat(sambanova): add MiniMax-M3, drop M2.5#2418

Open
octo-patch wants to merge 1 commit into
The-PR-Agent:mainfrom
octo-patch:feature/upgrade-minimax-m3
Open

feat(sambanova): add MiniMax-M3, drop M2.5#2418
octo-patch wants to merge 1 commit into
The-PR-Agent:mainfrom
octo-patch:feature/upgrade-minimax-m3

Conversation

@octo-patch
Copy link
Copy Markdown

Summary

Refresh the SambaNova MiniMax lineup so the listed defaults match what people are currently picking.

  • Add sambanova/MiniMax-M3 to MAX_TOKENS with a 192k context (same conservative cap we already use for M2.7), placed right above the M2.7 entry so it's the first SambaNova MiniMax option in the list.
  • Drop sambanova/MiniMax-M2.5 — it has been superseded by M2.7 / M3 and was just noise in the model list.
  • Keep sambanova/MiniMax-M2.7 as a fallback option for anyone who still wants the older model.
  • Update the SambaNova snippet in docs/docs/usage-guide/changing_a_model.md to use M3 as the primary model and M2.7 as the fallback in the fallback_models example.
  • Update the SambaNova key-forwarding test in test_litellm_api_key_guard.py to use sambanova/MiniMax-M3 so the test exercises the model id we now recommend. The test still verifies the same key-forwarding behavior.

Notes

  • No API URL or provider plumbing changes — only the model id list, the docs example, and the test fixture string.
  • 192k for M3 mirrors the value already in place for M2.7. SambaNova's M3 deployment may eventually expose a larger window, but matching M2.7 keeps this PR conservative; a follow-up can bump it once the deployed limit is confirmed.

Test plan

  • pr_agent/algo/__init__.py parses cleanly and MAX_TOKENS contains both sambanova/MiniMax-M3 and sambanova/MiniMax-M2.7, with M2.5 removed.
  • M3 entry appears before M2.7 in the dict so it is the first SambaNova MiniMax model listed.
  • pytest tests/unittest/test_litellm_api_key_guard.py::TestApiKeyGuard::test_sambanova_key_forwarded_for_non_ollama_model (CI to run; the test only swapped a model id string).

- Add `sambanova/MiniMax-M3` (192k context) to `MAX_TOKENS`, placed above
  M2.7 so it shows up as the recommended SambaNova model.
- Remove `sambanova/MiniMax-M2.5` from `MAX_TOKENS`; M2.5 is superseded by
  M2.7/M3 and the listing was getting stale.
- Update the SambaNova snippet in `changing_a_model.md` to use M3 as the
  primary model with M2.7 as the fallback.
- Switch the SambaNova key-forwarding unit test to `sambanova/MiniMax-M3`
  so the test exercises the recommended model id.
@github-actions github-actions Bot added the feature 💡 label Jun 2, 2026
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Upgrade SambaNova MiniMax lineup to M3 with M2.7 fallback

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add sambanova/MiniMax-M3 model with 192k context to MAX_TOKENS
• Remove superseded sambanova/MiniMax-M2.5 from model registry
• Update documentation to recommend M3 as primary model
• Update test to use M3 for key-forwarding verification
Diagram
flowchart LR
  A["SambaNova Model Registry"] -->|Add M3| B["MiniMax-M3 192k"]
  A -->|Remove M2.5| C["MiniMax-M2.5 removed"]
  A -->|Keep M2.7| D["MiniMax-M2.7 192k"]
  E["Documentation"] -->|Update example| F["M3 primary, M2.7 fallback"]
  G["Unit Test"] -->|Update model id| H["test_sambanova_key_forwarded"]

Loading

Grey Divider

File Changes

1. pr_agent/algo/__init__.py ✨ Enhancement +1/-1

Update SambaNova model registry with M3

• Add sambanova/MiniMax-M3 entry with 192k token limit
• Position M3 above M2.7 to make it the first SambaNova MiniMax option
• Remove sambanova/MiniMax-M2.5 entry (160k tokens)

pr_agent/algo/init.py


2. tests/unittest/test_litellm_api_key_guard.py 🧪 Tests +1/-1

Update SambaNova test to use MiniMax-M3

• Update test_sambanova_key_forwarded_for_non_ollama_model to use sambanova/MiniMax-M3
• Test continues to verify key-forwarding behavior with recommended model id

tests/unittest/test_litellm_api_key_guard.py


3. docs/docs/usage-guide/changing_a_model.md 📝 Documentation +3/-3

Update SambaNova documentation to recommend M3

• Update SambaNova configuration example to use sambanova/MiniMax-M3 as primary model
• Change fallback model from M2.5 to sambanova/MiniMax-M2.7
• Update section heading to reference M3 instead of M2.7

docs/docs/usage-guide/changing_a_model.md


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects Bot commented Jun 2, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0)

Grey Divider


Action required

1. Removed model breaks configs 🐞 Bug ☼ Reliability
Description
By deleting sambanova/MiniMax-M2.5 from MAX_TOKENS, any existing deployment still configured
with that model id will now raise an exception at runtime when token budgeting calls
get_max_tokens(). The default custom_model_max_tokens is non-positive, so there is no fallback
path and the agent fails fast.
Code

pr_agent/algo/init.py[230]

Evidence
The runtime token budget code raises for unknown models, and default settings do not provide a
positive custom token override, so removing a previously-known model id from MAX_TOKENS turns
existing configs into immediate failures.

pr_agent/algo/utils.py[992-1013]
pr_agent/settings/configuration.toml[30-35]
pr_agent/algo/init.py[226-233]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`MAX_TOKENS` no longer contains `sambanova/MiniMax-M2.5`, but the codebase treats missing models as a hard error. This creates a breaking change for any user config that still references the removed model id (either as `config.model` or in `fallback_models`).

## Issue Context
`get_max_tokens(model)` raises if the model is not in `MAX_TOKENS` and `config.custom_model_max_tokens` is not positive. The default configuration sets `custom_model_max_tokens=-1`, so most users will hit the exception path.

## Fix Focus Areas
Choose one of these approaches:
- Re-add `sambanova/MiniMax-M2.5` into `MAX_TOKENS` (optionally comment as deprecated), so existing configs continue to work.
- Or add an alias/migration in `get_max_tokens()` (and any other model-normalization code) that maps `sambanova/MiniMax-M2.5` -> `sambanova/MiniMax-M2.7` (or keeps the old 160k cap) while preserving new recommended defaults.

References:
- pr_agent/algo/__init__.py[226-233]
- pr_agent/algo/utils.py[992-1013]
- pr_agent/settings/configuration.toml[30-35]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread pr_agent/algo/__init__.py
'groq/llama-3.1-8b-instant': 128000,
'sambanova/MiniMax-M3': 192000,
'sambanova/MiniMax-M2.7': 192000,
'sambanova/MiniMax-M2.5': 160000,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Removed model breaks configs 🐞 Bug ☼ Reliability

By deleting sambanova/MiniMax-M2.5 from MAX_TOKENS, any existing deployment still configured
with that model id will now raise an exception at runtime when token budgeting calls
get_max_tokens(). The default custom_model_max_tokens is non-positive, so there is no fallback
path and the agent fails fast.
Agent Prompt
## Issue description
`MAX_TOKENS` no longer contains `sambanova/MiniMax-M2.5`, but the codebase treats missing models as a hard error. This creates a breaking change for any user config that still references the removed model id (either as `config.model` or in `fallback_models`).

## Issue Context
`get_max_tokens(model)` raises if the model is not in `MAX_TOKENS` and `config.custom_model_max_tokens` is not positive. The default configuration sets `custom_model_max_tokens=-1`, so most users will hit the exception path.

## Fix Focus Areas
Choose one of these approaches:
- Re-add `sambanova/MiniMax-M2.5` into `MAX_TOKENS` (optionally comment as deprecated), so existing configs continue to work.
- Or add an alias/migration in `get_max_tokens()` (and any other model-normalization code) that maps `sambanova/MiniMax-M2.5` -> `sambanova/MiniMax-M2.7` (or keeps the old 160k cap) while preserving new recommended defaults.

References:
- pr_agent/algo/__init__.py[226-233]
- pr_agent/algo/utils.py[992-1013]
- pr_agent/settings/configuration.toml[30-35]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@octo-patch please bring back this model 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants