Skip to content

fix(acp): split custom:name/model and provider/model model names (#53545) - #53558

Open
Tranquil-Flow wants to merge 1 commit into
NousResearch:mainfrom
Tranquil-Flow:fix/53545-acp-model-name-slash-split
Open

fix(acp): split custom:name/model and provider/model model names (#53545)#53558
Tranquil-Flow wants to merge 1 commit into
NousResearch:mainfrom
Tranquil-Flow:fix/53545-acp-model-name-slash-split

Conversation

@Tranquil-Flow

Copy link
Copy Markdown
Contributor

What

Fix model name parsing in ACP's _resolve_model_selection to correctly split provider/model when separated by / (in addition to existing colon : syntax).

Two bugs addressed (#53545):

  1. custom:name/model — Older code handled custom:name:model (colon) via parse_model_input, but external ACP clients like Multica send custom:zai/glm-5.2 (slash). Model name carried through as zai/glm-5.2 → API rejected. Now split when name matches a configured custom provider.

  2. provider/model — Native providers sent as zai/glm-5.2 (no custom: prefix) fell back to default provider with the whole string as model name. Now split when left side is a known provider, right side is non-empty, no colon present, and current provider is not an aggregator (guards OpenRouter's vendor/model slugs).

How Verified

  • RED→GREEN: 3 bug-specific regression tests fail on upstream/main, pass with the fix
  • Edge cases: OpenRouter slug preservation, unknown custom provider passthrough, empty input, default-model passthrough — all tested
  • Integration: set_session_model end-to-end test confirms provider routing
  • Full suite: 86/86 ACP server tests pass (no regressions)
  • Consistency: Both custom:zai/glm-5.2 (slash) and custom:zai:glm-5.2 (colon) now yield (custom:zai, glm-5.2)

Files

  • acp_adapter/server.py — two pre-parse branches + _is_configured_custom_provider helper
  • tests/acp/test_server.py — 6 new tests in TestModelNameSlashSplit

Competitor Check

No open PRs reference #53545. No Tranquil-Flow PRs. Topic-overlap search clean.

Auto-published by Moonsong via Path B automated pipeline.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused ACP fix. The reported behavior is still present on the inspected main: acp_adapter/server.py:646-662 delegates model parsing to parse_model_input, while hermes_cli/models.py:1739-1755 recognizes only colon-delimited provider syntax. The proposed pre-parse handles the two slash forms before that fallback and retains the existing aggregator guard using _AGGREGATOR_PROVIDERS (hermes_cli/models.py:1831-1833).

No correctness issue was identified from source and diff review. Both ACP switch paths share this resolver (acp_adapter/server.py:1773, acp_adapter/server.py:2002), and the PR adds direct coverage for named custom providers, native providers, unknown custom names, default-model pass-through, and OpenRouter slugs.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@Tranquil-Flow
Tranquil-Flow force-pushed the fix/53545-acp-model-name-slash-split branch from 1107d27 to 0ff0016 Compare July 17, 2026 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/acp Agent Communication Protocol adapter P3 Low — cosmetic, nice to have provider/zai ZAI provider sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform 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.

3 participants