fix(model-normalize): strip base-vendor prefix for regional-variant providers (#12140)#12165
fix(model-normalize): strip base-vendor prefix for regional-variant providers (#12140)#12165briandevans wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes model normalization for regional-variant providers (e.g., minimax-cn, kimi-coding-cn) so that a base-vendor vendor/model prefix (like minimax/... or moonshot/...) is stripped when targeting the regional variant, preventing upstream “unknown model” errors.
Changes:
- Add an explicit
_REGIONAL_VARIANT_ROOTSmapping to relate regional variants to their base vendors for prefix stripping. - Extend
_strip_matching_provider_prefixto strip base-vendor prefixes when the target is a configured regional variant. - Add focused regression tests covering the new behavior plus canaries to prevent widening/narrowing and to pin asymmetry.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
hermes_cli/model_normalize.py |
Introduces an explicit regional-variant→base-vendor mapping and applies it in provider-prefix stripping logic. |
tests/hermes_cli/test_model_normalize.py |
Adds regression coverage for Issue #12140, including preserved-behavior canaries and an asymmetry pin. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
CI audit — all 10 Classification:
None touch Focused suite: Other checks green: Reproduction commands on clean |
1fbd113 to
55f905e
Compare
|
Rebased onto current Re-ran focused tests on rebased head (
Ready for re-review whenever someone has bandwidth. |
8c8ac22 to
ef89e43
Compare
ef89e43 to
5843493
Compare
…roviders (NousResearch#12140) ``normalize_model_for_provider`` stripped a ``vendor/`` prefix from a model name only when the prefix matched the canonical target provider id. Regional-variant providers (``minimax-cn``, ``kimi-coding-cn``) — which route to the China region of the same vendor — have a different canonical id from the base vendor, so a natural config like model: provider: minimax-cn default: minimax/minimax-m2.7 reached the MiniMax-CN API with the ``minimax/`` prefix intact and produced: unknown model 'minimax/minimax-m2.7' (2013) The same shape breaks ``kimi-coding-cn`` for ``kimi/…`` / ``moonshot/…`` prefixed IDs. Reporter: NousResearch#12140. Root cause ---------- ``_strip_matching_provider_prefix`` (hermes_cli/model_normalize.py): normalized_prefix = _normalize_provider_alias(prefix) # "minimax" normalized_target = _normalize_provider_alias(target) # "minimax-cn" if normalized_prefix == normalized_target: # False return remainder return model_name # slash preserved The existing ``_PROVIDER_ALIASES`` map (``minimax-china``, ``minimax_cn`` → ``minimax-cn``) handles *regional-target aliases*, but there's no relationship expressing "the base vendor ``minimax`` is an acceptable prefix for the regional target ``minimax-cn``". Fix --- Add a ``_REGIONAL_VARIANT_ROOTS`` map: _REGIONAL_VARIANT_ROOTS = { "minimax-cn": "minimax", "kimi-coding-cn": "kimi-coding", } ``_strip_matching_provider_prefix`` now tries the existing equality check first (unchanged), then falls through to: *if target is a regional variant and the prefix resolves to its base vendor, strip it.* Any other prefix-target pair is still preserved — no widening of the strip behaviour for unrelated vendors, no symmetric "strip ``minimax-cn/`` for ``minimax`` target" rule (verified with an explicit test that pins that asymmetry). Behaviour matrix (resolved after this fix) ------------------------------------------ | Input | Target | Before | After | | --- | --- | --- | --- | | ``minimax/minimax-m2.7`` | ``minimax-cn`` | slash preserved | ``minimax-m2.7`` | | ``MiniMax/MiniMax-M2.7`` | ``minimax-cn`` | slash preserved | ``MiniMax-M2.7`` | | ``kimi/kimi-k2`` | ``kimi-coding-cn`` | slash preserved | ``kimi-k2`` | | ``moonshot/kimi-k2`` | ``kimi-coding-cn`` | slash preserved | ``kimi-k2`` | | ``kimi-coding/kimi-k2`` | ``kimi-coding-cn`` | slash preserved | ``kimi-k2`` | | ``arbitrary/path`` | ``minimax-cn`` | unchanged | **unchanged** | | ``minimax/foo`` | ``minimax`` | already stripped | **unchanged** | | ``zai/glm-5.1`` | ``zai`` | already stripped | **unchanged** | | ``anthropic/claude-sonnet-4.6`` | ``openrouter`` | preserved (aggregator) | **unchanged** | | ``minimax-cn/x`` | ``minimax`` | preserved (no symmetric rule) | **unchanged** | Narrow scope — explicitly not changed ------------------------------------- * Case normalisation. The reporter also suggested that ``minimax-m2.7`` (lowercase) should be rewritten to ``MiniMax-M2.7`` (PascalCase) for the MiniMax-CN API. That's a config-authoring issue (``hermes_cli/models.py:192-200`` lists the canonical PascalCase form), and adding a case-lookup table for every MiniMax generation is scope creep. The prefix-strip fix resolves the symptomatic ``unknown model 'minimax/...'`` error directly; any remaining case-sensitivity is a separate, smaller fix. * ``_resolve_auto`` in ``agent/auxiliary_client.py``. The reporter's "Fix 1" proposal was to normalise the model there, but the bug is actually at the normaliser layer — ``_resolve_auto`` *does* route through ``resolve_provider_client`` which already calls ``_normalize_resolved_model`` at every provider branch. Fixing ``_strip_matching_provider_prefix`` also repairs every other caller of ``normalize_model_for_provider`` for regional-variant providers (``switch_model``, fallback-model resolution, CLI picker), not just auxiliary. * launchd ``.env`` loading (reporter's "Fix 2"). Install-script / docs territory — mission-de-prioritised and orthogonal to this bug. Regression coverage ------------------- ``tests/hermes_cli/test_model_normalize.py`` gets a new ``TestIssue12140RegionalVariantPrefixStripping`` class with 17 parametrised cases (3 minimax-cn + 3 kimi-coding-cn + 10 preserved- behaviour canaries + 1 asymmetry pin). 7 of them fail on clean ``origin/main`` with the exact reporter symptom (``assert 'minimax/minimax-m2.7' == 'minimax-m2.7'``), confirming the regression is caught. The remaining 10 pin preserved behaviour so a future refactor can't accidentally widen or narrow the strip rule. Validation ---------- ``source venv/bin/activate && python -m pytest tests/hermes_cli/test_model_normalize.py -q`` -> 72 passed (55 pre-existing + 17 new). CI-aligned broad suite: 12909 passed, 39 skipped, 24 pre-existing baseline failures — none in ``hermes_cli/model_normalize.py`` or its test file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
5843493 to
3edc966
Compare
|
Closing to focus the queue on security/file-safety work where civilian merges are landing. Happy to reopen if maintainers want this picked up. |
Fixes #12140.
TL;DR
minimax-cn(andkimi-coding-cn) are regional variants ofminimax(andkimi-coding).normalize_model_for_provideronly strips avendor/prefix when the prefix matches the canonical target provider id, so a natural config likereaches the MiniMax-CN API with the
minimax/still attached and returnsunknown model 'minimax/minimax-m2.7' (2013).This PR teaches
_strip_matching_provider_prefixthat the base vendor is an acceptable prefix for its regional variant — only in that direction, and only for explicitly enumerated variants.Root cause
hermes_cli/model_normalize.py::_strip_matching_provider_prefix:The existing
_PROVIDER_ALIASESmap handles target aliases (minimax-china,minimax_cn→minimax-cn), but there is no relationship expressing "the base vendorminimaxis also an acceptable prefix for the regional targetminimax-cn."Fix
Add a narrow, explicit map:
_strip_matching_provider_prefixkeeps its existing equality check first (unchanged path), then falls through to: if the target is a regional variant and the prefix resolves to its base vendor, strip it. Every other prefix/target combination behaves exactly as before — no widening of the strip rule for unrelated vendors, and no symmetric "stripminimax-cn/for targetminimax" rule (pinned with an explicit test).Behaviour matrix
minimax/minimax-m2.7minimax-cnminimax-m2.7MiniMax/MiniMax-M2.7minimax-cnMiniMax-M2.7kimi/kimi-k2kimi-coding-cnkimi-k2moonshot/kimi-k2kimi-coding-cnkimi-k2kimi-coding/kimi-k2kimi-coding-cnkimi-k2arbitrary/pathminimax-cnminimax/foominimaxzai/glm-5.1zaianthropic/claude-sonnet-4.6openrouterminimax-cn/xminimaxNarrow scope — explicitly not changed
minimax-m2.7→MiniMax-M2.7for the MiniMax-CN API. That's a config-authoring concern (hermes_cli/models.py:192-200documents the canonical PascalCase form) and a case-lookup table per model generation is scope creep. The prefix-strip fix resolves the symptomaticunknown model 'minimax/...'error directly; remaining case-sensitivity is a separate, much smaller fix._resolve_autoinagent/auxiliary_client.py(reporter's "Fix 1"). The bug is at the normaliser layer, not at the auxiliary caller —_resolve_autoalready routes throughresolve_provider_clientwhich calls_normalize_resolved_modelon every provider branch. Fixing_strip_matching_provider_prefixalso repairsswitch_model, fallback-model resolution, and the CLI picker for regional-variant providers, not just auxiliary..envloading (reporter's "Fix 2"). Install-script / docs territory, orthogonal to this bug.Regression coverage
tests/hermes_cli/test_model_normalize.pygets a newTestIssue12140RegionalVariantPrefixStrippingclass with 17 parametrised cases:minimax-cnstrip cases (lowercase / PascalCase / mixed-case)kimi-coding-cnstrip cases (kimi/,moonshot/,kimi-coding/prefixes)arbitrary/,openrouter's aggregator form, case sensitivity on preserved remainders, etc.)minimax-cn/xis not stripped for targetminimax)7 of those fail on clean
origin/mainwith the exact reporter symptom:The remaining 10 pin preserved behaviour so a future refactor can't accidentally widen or narrow the strip rule.
Validation
CI-aligned broad suite: 12909 passed, 39 skipped, 24 pre-existing baseline failures — zero in
hermes_cli/model_normalize.pyor its test module. Happy to post the per-failure baseline audit here if it's useful.Pre-empted review questions
Q. Why a module-level dict instead of reusing
_PROVIDER_ALIASES?Different semantic.
_PROVIDER_ALIASESmaps alternate spellings of the same target (minimax-china→minimax-cn)._REGIONAL_VARIANT_ROOTSmaps a regional target to its different base vendor. Folding the two would make the asymmetry (root acceptable for variant, but not vice versa) invisible at the callsite.Q. Should every
*-cn/*-chinatarget get this treatment?No — the map is explicit by design. Aggregators like
openrouterandtogetherusevendor/modelas a routing prefix that must be preserved. An opt-in dict makes it impossible for a future provider addition to accidentally inherit strip behaviour.Q. Why not make the rule symmetric?
Intentional.
minimax-cn/modelsent to targetminimaxwould be a config error (user picked a different region's model id). Silent stripping would hide that; the explicit asymmetry test (test_minimax_cn_prefix_preserved_for_minimax_target) pins this for future refactors.Co-authored via LLM assistance; I've reviewed every line and am responsible for correctness.