Skip to content

fix(model-normalize): strip base-vendor prefix for regional-variant providers (#12140)#12165

Closed
briandevans wants to merge 1 commit into
NousResearch:mainfrom
briandevans:fix/minimax-cn-aux-auto-normalize
Closed

fix(model-normalize): strip base-vendor prefix for regional-variant providers (#12140)#12165
briandevans wants to merge 1 commit into
NousResearch:mainfrom
briandevans:fix/minimax-cn-aux-auto-normalize

Conversation

@briandevans

Copy link
Copy Markdown
Contributor

Fixes #12140.

TL;DR

minimax-cn (and kimi-coding-cn) are regional variants of minimax (and kimi-coding). normalize_model_for_provider only strips a vendor/ prefix when the prefix matches the canonical target provider id, so a natural config like

model:
  provider: minimax-cn
  default: minimax/minimax-m2.7

reaches the MiniMax-CN API with the minimax/ still attached and returns unknown model 'minimax/minimax-m2.7' (2013).

This PR teaches _strip_matching_provider_prefix that 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:

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 → API 400

The existing _PROVIDER_ALIASES map handles target aliases (minimax-china, minimax_cnminimax-cn), but there is no relationship expressing "the base vendor minimax is also an acceptable prefix for the regional target minimax-cn."

Fix

Add a narrow, explicit map:

_REGIONAL_VARIANT_ROOTS: dict[str, str] = {
    "minimax-cn": "minimax",
    "kimi-coding-cn": "kimi-coding",
}

_strip_matching_provider_prefix keeps 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 "strip minimax-cn/ for target minimax" rule (pinned with an explicit test).

Behaviour matrix

Input Target Before After
minimax/minimax-m2.7 minimax-cn slash preserved → API 400 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 unchanged (asymmetry pinned)

Narrow scope — explicitly not changed

  • Case normalisation. The reporter also suggests rewriting minimax-m2.7MiniMax-M2.7 for the MiniMax-CN API. That's a config-authoring concern (hermes_cli/models.py:192-200 documents the canonical PascalCase form) and a case-lookup table per model generation is scope creep. The prefix-strip fix resolves the symptomatic unknown model 'minimax/...' error directly; remaining case-sensitivity is a separate, much smaller fix.
  • _resolve_auto in agent/auxiliary_client.py (reporter's "Fix 1"). The bug is at the normaliser layer, not at the auxiliary caller — _resolve_auto already routes through resolve_provider_client which calls _normalize_resolved_model on every provider branch. Fixing _strip_matching_provider_prefix also repairs switch_model, fallback-model resolution, and the CLI picker for regional-variant providers, not just auxiliary.
  • launchd .env loading (reporter's "Fix 2"). Install-script / docs territory, 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 strip cases (lowercase / PascalCase / mixed-case)
  • 3 kimi-coding-cn strip cases (kimi/, moonshot/, kimi-coding/ prefixes)
  • 10 preserved-behaviour canaries (preserves arbitrary/, openrouter's aggregator form, case sensitivity on preserved remainders, etc.)
  • 1 asymmetry pin (minimax-cn/x is not stripped for target minimax)

7 of those fail on clean origin/main with the exact reporter symptom:

AssertionError: assert 'minimax/minimax-m2.7' == 'minimax-m2.7'

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 — zero in hermes_cli/model_normalize.py or 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_ALIASES maps alternate spellings of the same target (minimax-chinaminimax-cn). _REGIONAL_VARIANT_ROOTS maps 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 / *-china target get this treatment?
No — the map is explicit by design. Aggregators like openrouter and together use vendor/model as 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/model sent to target minimax would 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.

Copilot AI review requested due to automatic review settings April 18, 2026 14:07

Copilot AI left a comment

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.

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_ROOTS mapping to relate regional variants to their base vendors for prefix stripping.
  • Extend _strip_matching_provider_prefix to 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.

@briandevans

Copy link
Copy Markdown
Contributor Author

CI audit — all 10 test failures are pre-existing baselines, zero in touched code.

Classification:

Test Class Evidence
test_browser_camofox_state.py::test_config_version_matches_current_schema Version-pin drift Fails on clean origin/main at 6fb69229 locally (assert 19 == 18). Hard-coded schema-version assertion vs. current schema.
test_web_server.py::test_no_single_field_categories Schema category-count drift Fails on clean origin/main (Category 'code_execution' has only 1 field(s)).
test_concurrent_interrupt.py::test_concurrent_interrupt_cancels_pending _Stub missing method Fails on clean origin/main (AttributeError: '_Stub' object has no attribute '_apply_pending_steer_to_tool_results'). Test stub hasn't kept up with run_agent.py:8092 refactor.
test_concurrent_interrupt.py::test_running_concurrent_worker_sees_is_interrupted Same _Stub issue Same baseline AttributeError.
test_update_check.py::test_get_update_result_timeout pytest-xdist flake Timing-sensitive (assert 4775 is None). Passes serially + under -n auto locally on clean main.
test_send_message_tool.py::test_cron_different_target_still_sends pytest-xdist ordering Passes under -n auto locally on clean main — flake under CI worker scheduling.
test_send_message_tool.py::test_cron_same_chat_different_thread_still_sends Same ordering flake Passes locally under -n auto.
test_send_message_tool.py::test_sends_to_explicit_telegram_topic_target Same ordering flake Passes locally under -n auto.
test_send_message_tool.py::test_media_only_message_uses_placeholder_for_mirroring Same ordering flake Passes locally under -n auto.
test_skills_tool.py::test_gateway_load_returns_guidance_without_secret_capture pytest-xdist import-order Passes locally under -n auto.

None touch hermes_cli/model_normalize.py, tests/hermes_cli/test_model_normalize.py, or any call-path of _strip_matching_provider_prefix / normalize_model_for_provider.

Focused suite: pytest tests/hermes_cli/test_model_normalize.py -q72 passed (55 pre-existing + 17 new).

Other checks green: check-attribution, build-and-push, e2e, nix (ubuntu-latest), nix (macos-latest), supply-chain scan.

Reproduction commands on clean origin/main (6fb69229) available on request.

@briandevans

Copy link
Copy Markdown
Contributor Author

Rebased onto current origin/main (de9238d37) — was 12 days stale. Conflict was additive only: tests/hermes_cli/test_model_normalize.py had grown a sibling TestDeepseekVSeriesPassThrough / TestDeepseekCanonicalAndReasonerMapping block (PR #15066-era DeepSeek V-series fix); my TestIssue12140RegionalVariantPrefixStripping block was added below it. Production fix in hermes_cli/model_normalize.py (_REGIONAL_VARIANT_ROOTS map + _strip_matching_provider_prefix lookup) applied unchanged — verified the regional-variant prefix-stripping is still absent on origin/main, so this PR's gap is real.

Re-ran focused tests on rebased head (55f905e30):

Ready for re-review whenever someone has bandwidth.

@briandevans briandevans force-pushed the fix/minimax-cn-aux-auto-normalize branch 3 times, most recently from 8c8ac22 to ef89e43 Compare May 24, 2026 09:14
@briandevans briandevans force-pushed the fix/minimax-cn-aux-auto-normalize branch from ef89e43 to 5843493 Compare May 27, 2026 14:09
…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>
@briandevans briandevans force-pushed the fix/minimax-cn-aux-auto-normalize branch from 5843493 to 3edc966 Compare May 29, 2026 17:10
@briandevans

Copy link
Copy Markdown
Contributor Author

Closing to focus the queue on security/file-safety work where civilian merges are landing. Happy to reopen if maintainers want this picked up.

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

Labels

comp/agent Core agent loop, run_agent.py, prompt builder P2 Medium — degraded but workaround exists provider/minimax MiniMax (Anthropic transport) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Auxiliary compression fails with minimax-cn: model name not normalized in auto-detect

3 participants