feat(inference): add MiniMax provider - #6761
Conversation
Signed-off-by: octo-patch <266937838+octo-patch@users.noreply.github.com>
📝 WalkthroughWalkthroughMiniMax support is added across provider registration, inference configuration, onboarding menus, model selection, context-window handling, credentials, validation, installer help text, and associated tests. ChangesMiniMax provider integration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant SetupNim
participant ProviderConfig
participant InferenceConfig
participant ContextWindow
participant Environment
Operator->>SetupNim: select minimax and model
SetupNim->>ProviderConfig: resolve minimax configuration
ProviderConfig->>InferenceConfig: use minimax-api
SetupNim->>ContextWindow: apply known model context window
ContextWindow->>Environment: set NEMOCLAW_CONTEXT_WINDOW
InferenceConfig-->>SetupNim: return managed inference configuration
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Signed-off-by: octo-patch <266937838+octo-patch@users.noreply.github.com>
Signed-off-by: octo-patch <266937838+octo-patch@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/lib/onboard/setup-nim-flow.test.ts`:
- Around line 376-395: Update the test’s handleRemoteProviderSelection setup to
use the real production handler, or a delegating spy that invokes it, instead of
manually assigning MiniMax fields in the mock callback. Keep only external/input
boundaries such as menu selection mocked, and retain assertions through the
public createSetupNim flow so the test exercises the actual MiniMax
configuration and selection 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d4ea1b10-75bf-4e99-853a-bb67b34ca29b
📒 Files selected for processing (1)
src/lib/onboard/setup-nim-flow.test.ts
| const handleRemoteProviderSelection = vi.fn<SetupNimFlowDeps["handleRemoteProviderSelection"]>( | ||
| async ({ selected }, state) => { | ||
| expect(selected.key).toBe("minimax"); | ||
| state.model = "MiniMax-M3"; | ||
| state.provider = "minimax-api"; | ||
| state.endpointUrl = "https://api.minimax.io/v1"; | ||
| state.credentialEnv = "MINIMAX_API_KEY"; | ||
| return "selected"; | ||
| }, | ||
| ); | ||
| const setupNim = createSetupNim( | ||
| makeDeps({ | ||
| applyKnownModelContextWindow, | ||
| handleRemoteProviderSelection, | ||
| selectFromNumberedMenu: (_rawChoice, _defaultIndex, options) => { | ||
| const selected = options.find(({ key }) => key === "minimax"); | ||
| expect(selected).toBeDefined(); | ||
| return selected!; | ||
| }, | ||
| }), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Exercise the real remote-provider selection handler.
Line 376 replaces the behavior this test is meant to cover, and Lines 379-382 manually populate every MiniMax result field. The assertions then only prove that the mock wrote those values; they would still pass if the production MiniMax configuration or selection handler were broken. Use the real handler (or a delegating spy), with only external/input boundaries mocked.
As per path instructions, tests must verify observable behavior through the public boundary and must not use broad mocks that bypass the behavior under test.
🤖 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/lib/onboard/setup-nim-flow.test.ts` around lines 376 - 395, Update the
test’s handleRemoteProviderSelection setup to use the real production handler,
or a delegating spy that invokes it, instead of manually assigning MiniMax
fields in the mock callback. Keep only external/input boundaries such as menu
selection mocked, and retain assertions through the public createSetupNim flow
so the test exercises the actual MiniMax configuration and selection behavior.
Source: Path instructions
|
Thanks for taking the time to put this together. We intentionally treat the providers shown in NemoClaw onboarding as a curated and supported product surface, rather than a general provider registry. Additions there require prior product and maintainer alignment, a clear support commitment, and provider-specific validation. MiniMax is not currently part of that curated set, so we’re not going to accept this change. I’m closing the PR now so you don’t spend more time iterating on it. For future provider additions, please start with an issue or discussion so we can confirm the fit before implementation. Thanks again for the contribution. |
Summary
Testing
npx vitest run --project cli src/lib/domain/installer/provider.test.ts src/lib/inference/config.test.ts src/lib/inference/context-window.test.ts src/lib/inference/model-prompts.test.ts src/lib/onboard/provider-menu.test.ts src/lib/onboard/providers.test.ts src/lib/onboard/setup-nim-flow.test.ts src/lib/validation.test.tsnpx vitest run --project package-contract test/package-contract/onboard/compatible-endpoint-reasoning.test.tsnpm run build:clinpm run typecheck:clinpm run check:installer-hashnpm run check:diffSigned-off-by: octo-patch 266937838+octo-patch@users.noreply.github.com
Summary by CodeRabbit