feat: allow thinking_budget_tokens=0 for non-Anthropic providers (MiniMax/OpenAI-compatible support)#570
feat: allow thinking_budget_tokens=0 for non-Anthropic providers (MiniMax/OpenAI-compatible support)#570Lunox-1337 wants to merge 2 commits intoplastic-labs:mainfrom
Conversation
…upport) - Change gt=0 to ge=0 for THINKING_BUDGET_TOKENS in: - DeriverSettings (deriver) - SummarySettings (summary) - DreamSettings (dream) - MiniMax-M2.7 via OpenAI-compatible API does not support thinking budget parameter, so config must accept 0 - Existing client code already filters thinking_budget when provider != 'anthropic' (clients.py:1385)
WalkthroughChanged three settings' Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
🧹 Nitpick comments (1)
src/config.py (1)
259-259: Optional: mirror the Anthropic thinking-budget guard fromDialecticLevelSettings.
DialecticLevelSettings._validate_anthropic_thinking_budget(lines 338–349) rejects0 < THINKING_BUDGET_TOKENS < 1024for Anthropic (which Anthropic's API requires). Now thatDeriverSettings,SummarySettings, andDreamSettingsall acceptge=0, a user could still configure e.g.THINKING_BUDGET_TOKENS=512withPROVIDER="anthropic"and hit a runtime API error — this gap pre-dates this PR but is worth closing alongside the relaxation, particularly forSummarySettingswhosele=2000upper bound leaves a valid-but-API-invalid window of1..1023.Consider extracting the existing validator into a small helper (or mixin) and applying it to the three settings classes.
Also applies to: 441-441, 545-545
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.py` at line 259, Current config allows THINKING_BUDGET_TOKENS values (ge=0) that violate Anthropic's requirement (reject 0 < tokens < 1024); extract the existing DialecticLevelSettings._validate_anthropic_thinking_budget logic into a shared helper function or mixin (e.g., validate_anthropic_thinking_budget) and call it from DeriverSettings, SummarySettings, and DreamSettings validators so that when PROVIDER == "anthropic" those classes reject values 1..1023; update references to the THINKING_BUDGET_TOKENS field validation to invoke this shared validator to ensure consistent enforcement across settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/config.py`:
- Line 259: Current config allows THINKING_BUDGET_TOKENS values (ge=0) that
violate Anthropic's requirement (reject 0 < tokens < 1024); extract the existing
DialecticLevelSettings._validate_anthropic_thinking_budget logic into a shared
helper function or mixin (e.g., validate_anthropic_thinking_budget) and call it
from DeriverSettings, SummarySettings, and DreamSettings validators so that when
PROVIDER == "anthropic" those classes reject values 1..1023; update references
to the THINKING_BUDGET_TOKENS field validation to invoke this shared validator
to ensure consistent enforcement across settings.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/config.py (1)
359-395:⚠️ Potential issue | 🟠 MajorDefault dialectic levels hard-require an OpenAI-compatible endpoint with no graceful fallback.
All five reasoning levels default to
PROVIDER="custom"/MODEL="MiniMax-M2.7"with noBACKUP_PROVIDER/BACKUP_MODEL. Persrc/utils/clients.py:266-270,CLIENTS["custom"]is only registered when bothLLM_OPENAI_COMPATIBLE_API_KEYandLLM_OPENAI_COMPATIBLE_BASE_URLare set. Any deployment that upgrades without configuring both env vars will fail at dialectic request time withValueError("Missing client for custom")— a breaking change for existing Anthropic/Google users without a graceful degradation path.Consider one of:
- Add
BACKUP_PROVIDER="anthropic"+BACKUP_MODEL="claude-haiku-4-5"on each level for graceful fallback.- Revert to prior
anthropic/config.toml/docs.- Add a startup validator that errors with a clear message when any level uses
custombut bothLLM_OPENAI_COMPATIBLE_*env vars aren't set.The model identifier
"MiniMax-M2.7"is correct per the official MiniMax OpenAI-compatible API.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.py` around lines 359 - 395, The LEVELS default_factory currently assigns PROVIDER="custom"/MODEL="MiniMax-M2.7" for all DialecticLevelSettings which breaks deployments missing OpenAI-compatible env vars; update each DialecticLevelSettings entry in the LEVELS dict to include BACKUP_PROVIDER="anthropic" and BACKUP_MODEL="claude-haiku-4-5" so callers can gracefully fall back when CLIENTS["custom"] is not registered (see DialecticLevelSettings and CLIENTS behavior in src/utils/clients.py); ensure the added fields match the DialecticLevelSettings schema and update any validation if present to accept these backup fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/config.py`:
- Around line 359-395: The LEVELS default_factory currently assigns
PROVIDER="custom"/MODEL="MiniMax-M2.7" for all DialecticLevelSettings which
breaks deployments missing OpenAI-compatible env vars; update each
DialecticLevelSettings entry in the LEVELS dict to include
BACKUP_PROVIDER="anthropic" and BACKUP_MODEL="claude-haiku-4-5" so callers can
gracefully fall back when CLIENTS["custom"] is not registered (see
DialecticLevelSettings and CLIENTS behavior in src/utils/clients.py); ensure the
added fields match the DialecticLevelSettings schema and update any validation
if present to accept these backup fields.
|
Closed due to #459 |
2 participants
Summary
Changes the lower-bound validation for
THINKING_BUDGET_TOKENSfromgt=0(must be > 0) toge=0(must be >= 0) in three settings classes. This enables using Honcho with providers that do not support extended thinking/reasoning budgets — specifically MiniMax-M2.7 and other OpenAI-compatible APIs.Changes
config.py:259—DeriverSettings.THINKING_BUDGET_TOKENS:gt=0→ge=0config.py:441—SummarySettings.THINKING_BUDGET_TOKENS:gt=0→ge=0config.py:545—DreamSettings.THINKING_BUDGET_TOKENS:gt=0→ge=0Why
The existing code at
clients.py:1385already guards the thinking budget parameter — it is only passed to the API whenprovider == "anthropic"andthinking_budget_tokens > 0. However, the Pydantic validation was blockingthinking_budget_tokens=0at config load time, preventing Honcho from starting with providers that don't support the parameter.With this change, users can configure:
via the OpenAI-compatible API:
No Behaviour Change for Existing Users
ge=0(unchanged)thinking_budget_tokens > 0are unaffected — validation still requires values >= 0Summary by CodeRabbit
Refactor
Changes