Skip to content

fix(xai): migrate current model config slots - #31614

Open
ayooooo123 wants to merge 1 commit into
NousResearch:mainfrom
ayooooo123:fix/xai-current-model-migration
Open

fix(xai): migrate current model config slots#31614
ayooooo123 wants to merge 1 commit into
NousResearch:mainfrom
ayooooo123:fix/xai-current-model-migration

Conversation

@ayooooo123

Copy link
Copy Markdown

Summary

  • scan current Hermes model config slots (model.default / model.model) for retired xAI model IDs
  • support root string model: xai/grok-3 configs during migration
  • preserve explicit x-ai/ and xai/ provider prefixes when applying replacements

Test Plan

  • PYTHONPATH=$PWD python -m pytest tests/hermes_cli/test_xai_retirement.py tests/hermes_cli/test_migrate_xai.py -q
  • PYTHONPATH=$PWD python -m pytest tests/hermes_cli/test_xai_retirement.py tests/hermes_cli/test_migrate_xai.py tests/hermes_cli/test_auth_xai_oauth_provider.py tests/hermes_cli/test_xai_oauth_pkce_token_exchange.py tests/hermes_cli/test_tools_config.py tests/hermes_cli/test_runtime_provider_resolution.py -q
  • git diff --check origin/main..HEAD
  • static added-line scan for secrets / shell injection / eval / pickle / SQL string formatting: no findings
  • independent reviewer: passed; no blocking security or logic issues

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard provider/xai xAI (Grok) labels May 24, 2026

@teknium1 teknium1 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.

Thanks for covering the active primary-model config shape; that gap is real on current main.

Problems

  • The new model.model scan should follow the current resolver precedence. hermes_cli/runtime_provider.py:283-289 treats model.model as a fallback only when model.default is absent, and hermes_cli/config.py:6565-6575 canonicalizes the same default > model > name order. Scanning both can migrate an inactive alias and, for a non-reasoning alias, write reasoning_effort: none beside the active default.

Suggested changes

  • Make the legacy model.model check conditional on an absent/empty model.default, and add the corresponding precedence regression test. The root-string and provider-prefix apply cases can remain.

Automated hermes-sweeper review.


model_cfg = config.get("model")
if isinstance(model_cfg, dict):
_check("model.default", model_cfg.get("default"))

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.

model.model is a fallback alias, not a second active model slot: runtime_provider._get_model_config() uses it only when model.default is empty. Please make this an elif/fallback check and test that a valid default plus a retired legacy alias does not generate an alias migration or set reasoning_effort on the active model block.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists provider/xai xAI (Grok) sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants