fix(vlm): skip zhipu prefix when model already uses zai/ (LiteLLM)#789
fix(vlm): skip zhipu prefix when model already uses zai/ (LiteLLM)#789qin-ctx merged 2 commits intovolcengine:mainfrom
Conversation
When the model string already uses LiteLLM's zai/ prefix (e.g. zai/glm-4.5), do not prepend zhipu/. Otherwise LiteLLM receives zhipu/zai/... which it does not recognize. Fixes volcengine#784 Made-with: Cursor
|
|
There was a problem hiding this comment.
Pull request overview
Fixes LiteLLM VLM model name rewriting so Z.AI/GLM models that already use LiteLLM’s zai/ prefix are not incorrectly rewritten into invalid zhipu/zai/... model identifiers.
Changes:
- Update
_resolve_model()to skip prefixing when the model string already starts withzai/. - Add an inline comment documenting the Z.AI/LiteLLM prefix behavior and linking to #784.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| prefix = PROVIDER_CONFIGS[provider]["litellm_prefix"] | ||
| if prefix and not model.startswith(f"{prefix}/"): | ||
| # LiteLLM uses the `zai/` prefix for Zhipu GLM; do not prepend `zhipu/` (see #784). | ||
| if prefix and not model.startswith(f"{prefix}/") and not model.startswith("zai/"): |
There was a problem hiding this comment.
[Bug] (blocking) The not model.startswith("zai/") guard applies to every provider, not just zhipu. If any other provider's model happened to start with zai/, its prefix would be silently skipped. This was also flagged by the previous Copilot review and hasn't been addressed.
Minimal fix — scope to zhipu:
| if prefix and not model.startswith(f"{prefix}/") and not model.startswith("zai/"): | |
| # LiteLLM uses the `zai/` prefix for Zhipu GLM; do not prepend `zhipu/` (see #784). | |
| if prefix and not model.startswith(f"{prefix}/") and not (provider == "zhipu" and model.startswith("zai/")): |
Alternatively, encode this as provider-specific config so future alternative prefixes are declarative:
# in PROVIDER_CONFIGS["zhipu"]
"alt_prefixes": ["zai"],
# in _resolve_model
alt_prefixes = PROVIDER_CONFIGS[provider].get("alt_prefixes", [])
all_prefixes = [p for p in [prefix] + alt_prefixes if p]
if prefix and not any(model.startswith(f"{p}/") for p in all_prefixes):
return f"{prefix}/{model}"| prefix = PROVIDER_CONFIGS[provider]["litellm_prefix"] | ||
| if prefix and not model.startswith(f"{prefix}/"): | ||
| # LiteLLM uses the `zai/` prefix for Zhipu GLM; do not prepend `zhipu/` (see #784). | ||
| if prefix and not model.startswith(f"{prefix}/") and not model.startswith("zai/"): |
There was a problem hiding this comment.
[Suggestion] (non-blocking) No regression test for this fix. There are currently no tests for _resolve_model at all. Consider adding at least:
def test_resolve_model_zai_prefix_not_double_prefixed():
provider = LiteLLMVLMProvider({"model": "zai/glm-4.5", "provider": "litellm"})
assert provider._resolve_model("zai/glm-4.5") == "zai/glm-4.5"Made-with: Cursor
|
Updated. I scoped the |
Summary
Fixes incorrect LiteLLM model rewriting: when the configured model already uses LiteLLM's
zai/prefix (e.g.zai/glm-4.5), the VLM backend no longer prependszhipu/, which produced invalid names likezhipu/zai/glm-4.5.Context
Change
In
_resolve_model, skip adding the zhipu prefix if the model string already starts withzai/.Made with Cursor