fix: route MiniMax compacting through Anthropic-compatible API#1154
fix: route MiniMax compacting through Anthropic-compatible API#1154jatmn wants to merge 16 commits into
Conversation
Switch MiniMax provider setup away from the OpenAI-compatible shim and onto the Anthropic-compatible endpoint. Update env-only, provider flag, and saved profile paths to use ANTHROPIC_* while preserving legacy OPENAI_MODEL as a migration fallback. Adjust MiniMax M2 context metadata so shared descriptors use the gateway-safe 196608 window and the direct MiniMax catalog overrides to the documented 204800 window. Extend runtime context lookup to anthropic-proxy routes so compact budgeting uses the direct provider metadata. Update MiniMax client, provider profile, provider flag, context, and auto-compact tests for the Anthropic-compatible route and provider-specific compact limits.
Update ProviderManager test fixtures so MiniMax uses the Anthropic-compatible endpoint instead of the old OpenAI-compatible /v1 endpoint. Add coverage for the /provider add flow to assert MiniMax saves provider=minimax, endpoint https://api.minimax.io/anthropic, and displays the Anthropic-compatible API provider type. Add edit-flow coverage to ensure existing MiniMax profiles remain on the Anthropic-compatible provider path and continue hiding OpenAI-only advanced fields.
Harden MiniMax client and compact tests against ambient CI provider env such as OPENAI_API_KEY, ANTHROPIC_BASE_URL, and provider-profile markers. The compact budget test now explicitly clears competing provider flags before asserting direct MiniMax metadata, preventing CI-level OpenAI credentials from masking env-only MiniMax route detection.
Make each env-only MiniMax client test clear competing provider flags, OpenAI/XAI keys, Anthropic env, and saved-profile markers before setting MINIMAX_API_KEY. This keeps the Anthropic-compatible MiniMax route assertions independent of CI-level process env that can otherwise mask env-only provider detection in the full serial suite.
Route MiniMax env-only requests by explicit MiniMax model/base intent even when generic OpenAI-compatible environment variables are present, while preserving non-MiniMax base URL conflicts. Use the resolved MiniMax env-only path for Anthropic SDK key selection so stale provider classification or Bun module mocks cannot fall back to an Anthropic test key. Harden MiniMax compact coverage against leaked env overrides and prior autoCompact module mocks, and cover the ambient OpenAI/XAI env regression.
Restore Bun module mocks after compression test files so their deterministic autoCompact window does not leak into later compact tests in full-suite order. Verified the MiniMax compact regression now passes after the compression suites and in the full local test log.
Replace the compression suites' top-level Bun module mocks for autoCompact/config with real test config and env controls. This avoids Bun 1.3.11 leaking a mocked effective context window into the later MiniMax compact test in full-suite order. Verified compression-before-compact and MiniMax focused suites pass locally.
CI enables the output-token slot-reservation cap, so MiniMax's direct 204,800 context can produce a 196,800 effective compact window instead of the uncapped 184,800. Keep the test focused on direct MiniMax context metadata while accepting either reservation state.
|
@kevincodex1 testing of this should include a manual test of minimax using minimax api/subscription |
techbrewboss
left a comment
techbrewboss
left a comment
There was a problem hiding this comment.
Review summary
Thanks for the focused MiniMax routing work. The direction looks valuable: moving direct MiniMax traffic onto the Anthropic-compatible endpoint matches the compacting failure mode, and the client/profile coverage catches a lot of the intended behavior.
I found one startup precedence bug that should block merge, plus one smaller user-facing regression around the benchmark command.
Findings
-
src/utils/providerProfiles.ts:344- Recommended MiniMax env can be overridden by saved profiles.
Impact: This PR now documents MiniMax asMINIMAX_API_KEY+ANTHROPIC_BASE_URL+ANTHROPIC_MODELwith noCLAUDE_CODE_USE_*flag in.env.example. But startup profile application only treats flagged providers as explicit selections. If a user has any active saved provider profile, this env-only MiniMax config is not considered “complete”, soapplyActiveProviderProfileFromConfig()falls through and applies the saved profile over the explicit MiniMax env. That breaks the new documented setup for users who have provider profiles.
Suggested fix: Teach the startup gate to treatresolveEnvOnlyProviderRouteId(processEnv)or specificallyMINIMAX_API_KEYplus MiniMax route intent as an explicit provider selection, and add a regression test where env-only MiniMax is present alongside an active saved profile. -
src/utils/model/benchmark.ts:25- Benchmark support still assumes MiniMax is OpenAI-compatible.
Impact: After this PR, MiniMax no longer setsOPENAI_BASE_URL/OPENAI_API_KEY, but benchmark support still builds only OpenAI-compatible/chat/completionsrequests from those env vars. MiniMax will now be reported unsupported even thoughsrc/commands/benchmark.tsstill lists it as supported.
Suggested fix: Either update benchmark to support MiniMax throughANTHROPIC_BASE_URL+MINIMAX_API_KEYusing Anthropic message streaming semantics, or remove MiniMax from the supported-provider message.
Validation
Reviewed PR metadata and diff locally, including MiniMax route metadata, client setup, provider profile startup logic, integration descriptors, and nearby docs/tests. GitHub CI is currently green. I did not run the local test suite.
Treat env-only provider routes such as direct MiniMax as complete startup provider selections so saved profiles do not override explicit MINIMAX_API_KEY/ANTHROPIC_* env. Stop advertising direct MiniMax benchmark support through the OpenAI-compatible benchmark path, and add regression coverage for the unsupported direct MiniMax benchmark env.
techbrewboss
left a comment
techbrewboss
left a comment
There was a problem hiding this comment.
Re-review summary
Thanks for the follow-up. The latest commit addresses both findings from my previous review.
- Startup precedence:
hasCompleteProviderSelection()now treats env-only routes fromresolveEnvOnlyProviderRouteId()as explicit selections, and the new regression test covers MiniMax env not being overridden by a saved profile. - Benchmark support: direct MiniMax was removed from the OpenAI-compatible benchmark path/message, with focused coverage that the Anthropic-compatible MiniMax env is not advertised as benchmark-supported.
Validation
Ran the focused tests on the PR head:
bun test src/utils/providerProfiles.test.ts src/utils/model/benchmark.test.tsResult: 70 pass, 0 fail. GitHub checks are also green.
No remaining blockers from my review.
Recognize MiniMax when /provider loads it through the Anthropic-compatible env shape using ANTHROPIC_BASE_URL, ANTHROPIC_MODEL, and ANTHROPIC_API_KEY. Label MiniMax correctly on the startup screen and skip the Anthropic custom-key approval prompt when the resolved provider is not using the Anthropic account flow. Add regressions for route metadata, legacy provider classification, account-flow bypass, and startup display for Anthropic-compatible MiniMax profiles.
Resolve conflicts with main's Xiaomi MiMo provider additions while preserving MiniMax's direct Anthropic-compatible routing. Conflict areas resolved: route metadata provider precedence, API client env-only routing, OpenAI-shim provider model selection tests, and model provider classification. Validation: bun test --max-concurrency=1 src\integrations\routeMetadata.test.ts src\utils\model\providers.test.ts src\utils\model\model.openai-shim-providers.test.ts src\services\api\client.test.ts src\components\StartupScreen.test.ts; bun run build.
|
Real-world testing shows this works.. but the compact will need more stress testing if there are further issues. |
gnanam1990
left a comment
gnanam1990
left a comment
There was a problem hiding this comment.
Local checks on 7aa3b83:
bun test src/utils/providerProfiles.test.ts src/utils/model/benchmark.test.ts src/integrations/routeMetadata.test.ts— ✅ 99 pass / 0 fail (includes the regression test for env-only MiniMax not being overridden by a saved profile)bun test src/services/compact/autoCompact.test.ts src/services/api/compressToolHistory.test.ts src/services/api/openaiShim.compression.test.ts— ✅ 43 passbun test src/services/api/client.test.ts— 19 pass / 1 fail — the failing case (first-party Anthropic requests execute the configured fetch wrapper without runtime symbol errors) also fails onorigin/mainat the same commit, so it's a pre-existing flake unrelated to this PR.bun run build && node dist/cli.mjs --version— ✅0.10.0 (OpenClaude)
Code review:
- Endorse @techbrewboss's dismissed re-review — both prior findings are addressed:
hasCompleteProviderSelection()correctly treatsresolveEnvOnlyProviderRouteId()env-only routes as explicit selections, with a focused regression test.- MiniMax removed from the OpenAI-compatible benchmark supported list to match the new Anthropic-compatible routing.
- 27-file scope looks wide for a "fix" but is coherent: MiniMax routing change forces touches in
routeMetadata(catalog),client.ts(transport),providerProfiles.ts(startup precedence),benchmark.ts(capability advertising),.env.example(docs), plus their test coverage. No drive-by churn observed. - No red flags: no
tengu_*/ Anthropic fingerprints, no new network calls in 3P paths, no CI workflow diffs, no new deps.
Trusting @jatmn's real-world MiniMax test confirmation. LGTM.
techbrewboss
left a comment
techbrewboss
left a comment
There was a problem hiding this comment.
Review summary
Thanks for the MiniMax routing work. The implementation direction looks valuable: direct MiniMax now uses the Anthropic-compatible endpoint, the env/profile/provider-manager paths are covered, and the previous startup precedence and benchmark issues appear addressed.
I found one PR-specific typecheck issue that should be fixed before merge.
Findings
src/utils/providerProfile.ts:449-ANTHROPIC_API_KEYis not part ofSecretValueSource.
Impact: The PR addsANTHROPIC_API_KEY: keyto aSecretValueSourceobject, but that local type only allowsOPENAI_API_KEY,MINIMAX_API_KEY, and other non-Anthropic secret keys.bun run typecheckreportsTS2353on this changed line. The repo has many broader typecheck errors, but this one is directly introduced by the PR.
Suggested fix: AddANTHROPIC_API_KEYto the localSecretValueSourcekey union insrc/utils/providerProfile.ts, or avoid including it in that object ifOPENAI_API_KEY: keyis the intended redaction alias.
Validation
- Reviewed PR metadata, prior reviews, the OpenClaude review reference, and the MiniMax/provider diff.
bun test src/utils/providerProfiles.test.ts src/utils/providerFlag.test.ts src/utils/model/providers.test.ts src/integrations/routeMetadata.test.ts— 181 pass / 0 fail.bun run integrations:check— passed.bun run typecheck— failed with many existing repo errors, including the PR-specificsrc/utils/providerProfile.ts:449error above.
Recommendation: request changes until the new type error is fixed.
Allow MiniMax profile redaction to include ANTHROPIC_API_KEY in the SecretValueSource type used by sanitizeProviderConfigValue. This fixes the PR-specific TS2353 reported by review while keeping the MiniMax Anthropic-compatible key alias redacted alongside MINIMAX_API_KEY. Validation: bun test --max-concurrency=1 src\utils\providerProfiles.test.ts src\utils\providerFlag.test.ts src\utils\model\providers.test.ts src\integrations\routeMetadata.test.ts; bun run typecheck still has existing repo-wide errors, with no providerProfile.ts matches.
Resolve test harness conflicts from main's shared mutation lock updates while preserving MiniMax env cleanup and compression test configuration reset coverage. Conflict areas resolved: src/services/api/client.test.ts and src/services/api/openaiShim.compression.test.ts. Validation: bun test --max-concurrency=1 src\services\api\client.test.ts src\services\api\openaiShim.compression.test.ts src\utils\providerProfiles.test.ts src\utils\providerFlag.test.ts src\utils\model\providers.test.ts src\integrations\routeMetadata.test.ts; bun run build.
Add a narrow compression-enabled override for tests so the compression suites do not depend on shared global config state from the full Bun runner. Pass explicit effective context windows in direct compression tests and use catalog-backed models in shim compression tests to avoid env-capped tier drift. Verified with focused compression tests, full bun test --max-concurrency=1, and bun run build.
Verify the quarantined corrupted Orama file from the actual persistence directory returned by getOramaPersistencePath, instead of assuming the config-dir projects root used by the full CI runner. Verified with the failing KnowledgeGraph stress test, compression smoke suites, full bun test --max-concurrency=1, and bun run build.
BlockersNone. Non-Blocking
Looks Good
Verdict: Approve — clean MiniMax routing fix. |
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Clean MiniMax routing fix. All review comments addressed.
# Conflicts: # src/services/api/compressToolHistory.test.ts # src/services/api/openaiShim.compression.test.ts # src/services/compact/autoCompact.test.ts # src/utils/knowledgeGraph.stress.test.ts # src/utils/model/providers.test.ts
techbrewboss
left a comment
techbrewboss
left a comment
There was a problem hiding this comment.
Review summary
No blocking findings from this pass. The PR is valuable as implemented: it moves direct MiniMax from the OpenAI-compatible shim to the Anthropic-compatible endpoint, updates env/profile/provider-manager startup paths, and keeps benchmark support honest by no longer advertising MiniMax through the OpenAI-compatible benchmark flow.
Findings
No blocking findings.
Risk
No malicious or suspicious behavior found. I did not see dependency, workflow, install script, or package changes. The new network behavior is scoped to the stated MiniMax provider endpoint change.
Validation
bun test --max-concurrency=1 src/utils/providerProfiles.test.ts src/utils/providerFlag.test.ts src/utils/model/providers.test.ts src/integrations/routeMetadata.test.ts src/services/api/client.test.ts src/services/compact/autoCompact.test.ts src/utils/model/benchmark.test.ts— 205 passbun run integrations:check— passedbun run build— passed- Reviewed PR metadata, prior reviews, diff, docs/reference context, provider routing, profile env handling, client auth routing, compact context lookup, and changed-file security surface.
|
Did an independent pass focused on the red-flag surface and intent, since @techbrewboss already approved the current head and the change is large (29 files). The key thing I wanted to confirm given the title: "Anthropic-compatible" here routes MiniMax through MiniMax's own I scoped this to a red-flag + intent review rather than a full re-verification of all 891 lines, so I'm not adding a second approval on top of @techbrewboss's — just confirming the Anthropic-routing question is clean and recording that. Worth making sure CI / |
4 participants
Summary
https://api.minimax.io/anthropicANTHROPIC_*while preserving legacyOPENAI_MODELas a model fallback196_608, while overriding the direct MiniMax route to the documented204_800Impact
Testing
bun run buildopenclaude v0.10.0todist/cli.mjsdist/sdk.mjs56 exports match)/providerMiniMax coverage commit: passedbun run smoke0.10.0 (OpenClaude)/providerMiniMax coverage commit: passedbun test src\utils\context.test.ts src\services\compact\autoCompact.test.ts src\integrations\index.test.ts src\integrations\routeMetadata.test.ts src\utils\providerProfiles.test.ts src\utils\providerFlag.test.ts src\services\api\client.test.tsbun test src\components\ProviderManager.test.tsx src\utils\providerProfiles.test.ts src\commands\provider\provider.test.tsx/provideradd flow saves MiniMax withprovider: minimax, endpointhttps://api.minimax.io/anthropic, and provider typeAnthropic-compatible API/provideredit flow keeps MiniMax on the Anthropic-compatible provider path and hides OpenAI-only advanced fieldsNotes
204,800for the Anthropic-compatible API.197Kcontext, so the shared descriptor is kept at196,608and the direct MiniMax route carries the higher override.--provider minimaxoverwrites staleANTHROPIC_BASE_URL/ANTHROPIC_MODELinstead of preserving previous Anthropic values.