mirror of
https://github.com/crewAIInc/crewAI.git
synced 2026-08-10 08:21:54 +00:00
fix: preserve provider on LiteLLM-routed models (#6849)
* fix: preserve provider on LiteLLM-routed models LiteLLM construction computed the real provider in `__new__` but never passed it into init, so `BaseLLM` silently defaulted every shared-path model to `openai`. Infer the provider from a `provider/model` prefix when none is supplied so groq, cohere, mistral, and the rest report themselves correctly to callers like the policy engine. * fix: avoid double-prefixing instructor model strings With LiteLLM models now carrying a real `provider` while `model` keeps its `provider/name` form, `InternalInstructor` was building `groq/groq/...` for `instructor.from_provider`. Skip the prefix when the model string is already qualified. * fix: format LiteLLM multimodal content as OpenAI-shaped blocks Preserving the real provider on the LiteLLM path made `format_multimodal_content` emit Anthropic-native blocks for `anthropic/...` models, which LiteLLM rejects. Keep `provider` as the model identity for policies, but format multimodal blocks with the OpenAI chat schema when `is_litellm` is set. Expose the formatter helper on `BaseLLM` so native OpenAI/Azure completions share the same API.
This commit is contained in:
@@ -2225,14 +2225,14 @@ class LLM(BaseLLM):
|
|||||||
)
|
)
|
||||||
return messages
|
return messages
|
||||||
|
|
||||||
provider = self.provider or self.model
|
formatter = self._multimodal_formatter_name()
|
||||||
|
|
||||||
for msg in messages:
|
for msg in messages:
|
||||||
files = msg.get("files")
|
files = msg.get("files")
|
||||||
if not files:
|
if not files:
|
||||||
continue
|
continue
|
||||||
|
|
||||||
content_blocks = format_multimodal_content(files, provider)
|
content_blocks = format_multimodal_content(files, formatter)
|
||||||
if not content_blocks:
|
if not content_blocks:
|
||||||
msg.pop("files", None)
|
msg.pop("files", None)
|
||||||
continue
|
continue
|
||||||
@@ -2250,6 +2250,13 @@ class LLM(BaseLLM):
|
|||||||
|
|
||||||
return messages
|
return messages
|
||||||
|
|
||||||
|
def _multimodal_formatter_name(self) -> str:
|
||||||
|
# Identity (`self.provider`) stays e.g. anthropic. LiteLLM's completion()
|
||||||
|
# API is OpenAI-shaped and translates blocks to the vendor on the wire.
|
||||||
|
if self.is_litellm:
|
||||||
|
return "openai"
|
||||||
|
return self.provider or self.model
|
||||||
|
|
||||||
async def _aprocess_message_files(
|
async def _aprocess_message_files(
|
||||||
self, messages: list[LLMMessage]
|
self, messages: list[LLMMessage]
|
||||||
) -> list[LLMMessage]:
|
) -> list[LLMMessage]:
|
||||||
@@ -2276,14 +2283,14 @@ class LLM(BaseLLM):
|
|||||||
)
|
)
|
||||||
return messages
|
return messages
|
||||||
|
|
||||||
provider = self.provider or self.model
|
formatter = self._multimodal_formatter_name()
|
||||||
|
|
||||||
for msg in messages:
|
for msg in messages:
|
||||||
files = msg.get("files")
|
files = msg.get("files")
|
||||||
if not files:
|
if not files:
|
||||||
continue
|
continue
|
||||||
|
|
||||||
content_blocks = await aformat_multimodal_content(files, provider)
|
content_blocks = await aformat_multimodal_content(files, formatter)
|
||||||
if not content_blocks:
|
if not content_blocks:
|
||||||
msg.pop("files", None)
|
msg.pop("files", None)
|
||||||
continue
|
continue
|
||||||
|
|||||||
@@ -274,7 +274,10 @@ class BaseLLM(BaseModel, ABC):
|
|||||||
data["stop"] = list(stop)
|
data["stop"] = list(stop)
|
||||||
|
|
||||||
if not data.get("provider"):
|
if not data.get("provider"):
|
||||||
data["provider"] = "openai"
|
model = data.get("model") or ""
|
||||||
|
data["provider"] = (
|
||||||
|
cls._extract_provider(model) if isinstance(model, str) else "openai"
|
||||||
|
)
|
||||||
|
|
||||||
known_fields = set(cls.model_fields.keys())
|
known_fields = set(cls.model_fields.keys())
|
||||||
extras = {k: v for k, v in data.items() if k not in known_fields}
|
extras = {k: v for k, v in data.items() if k not in known_fields}
|
||||||
@@ -507,6 +510,10 @@ class BaseLLM(BaseModel, ABC):
|
|||||||
"""
|
"""
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
def _multimodal_formatter_name(self) -> str:
|
||||||
|
# Content-block schema key for crewai_files. Identity stays on self.provider.
|
||||||
|
return self.provider or self.model
|
||||||
|
|
||||||
def format_text_content(self, text: str) -> dict[str, Any]:
|
def format_text_content(self, text: str) -> dict[str, Any]:
|
||||||
"""Format text as a content block for the LLM.
|
"""Format text as a content block for the LLM.
|
||||||
|
|
||||||
@@ -866,7 +873,7 @@ class BaseLLM(BaseModel, ABC):
|
|||||||
)
|
)
|
||||||
return messages
|
return messages
|
||||||
|
|
||||||
provider = getattr(self, "provider", None) or getattr(self, "model", "openai")
|
formatter = self._multimodal_formatter_name()
|
||||||
api = getattr(self, "api", None)
|
api = getattr(self, "api", None)
|
||||||
|
|
||||||
for msg in messages:
|
for msg in messages:
|
||||||
@@ -878,7 +885,7 @@ class BaseLLM(BaseModel, ABC):
|
|||||||
text = existing_content if isinstance(existing_content, str) else None
|
text = existing_content if isinstance(existing_content, str) else None
|
||||||
|
|
||||||
content_blocks = format_multimodal_content(
|
content_blocks = format_multimodal_content(
|
||||||
files, provider, api=api, prefer_upload=self.prefer_upload, text=text
|
files, formatter, api=api, prefer_upload=self.prefer_upload, text=text
|
||||||
)
|
)
|
||||||
if not content_blocks:
|
if not content_blocks:
|
||||||
msg.pop("files", None)
|
msg.pop("files", None)
|
||||||
|
|||||||
@@ -105,7 +105,12 @@ class InternalInstructor(Generic[T]):
|
|||||||
if value is not None:
|
if value is not None:
|
||||||
extra_kwargs[attr] = value
|
extra_kwargs[attr] = value
|
||||||
|
|
||||||
return instructor.from_provider(f"{provider}/{model_string}", **extra_kwargs)
|
qualified_model = (
|
||||||
|
model_string
|
||||||
|
if not provider or model_string.startswith(f"{provider}/")
|
||||||
|
else f"{provider}/{model_string}"
|
||||||
|
)
|
||||||
|
return instructor.from_provider(qualified_model, **extra_kwargs)
|
||||||
|
|
||||||
def _extract_provider(self) -> str:
|
def _extract_provider(self) -> str:
|
||||||
"""Extract provider from LLM model name.
|
"""Extract provider from LLM model name.
|
||||||
|
|||||||
@@ -77,8 +77,8 @@ startxref
|
|||||||
|
|
||||||
def _build_multimodal_message(llm: LLM, prompt: str, files: dict) -> list[dict]:
|
def _build_multimodal_message(llm: LLM, prompt: str, files: dict) -> list[dict]:
|
||||||
"""Build a multimodal message with text and file content."""
|
"""Build a multimodal message with text and file content."""
|
||||||
provider = getattr(llm, "provider", None) or llm.model
|
formatter = llm._multimodal_formatter_name()
|
||||||
content_blocks = format_multimodal_content(files, provider)
|
content_blocks = format_multimodal_content(files, formatter)
|
||||||
return [
|
return [
|
||||||
{
|
{
|
||||||
"role": "user",
|
"role": "user",
|
||||||
|
|||||||
@@ -860,16 +860,19 @@ def test_prefixed_models_with_invalid_constants_use_litellm():
|
|||||||
llm = LLM(model="openai/gemini-2.5-flash", is_litellm=False)
|
llm = LLM(model="openai/gemini-2.5-flash", is_litellm=False)
|
||||||
assert llm.is_litellm is True
|
assert llm.is_litellm is True
|
||||||
assert llm.model == "openai/gemini-2.5-flash"
|
assert llm.model == "openai/gemini-2.5-flash"
|
||||||
|
assert llm.provider == "openai"
|
||||||
|
|
||||||
# Test openai/ prefix with model that doesn't match patterns (e.g. no gpt- prefix) → LiteLLM
|
# Test openai/ prefix with model that doesn't match patterns (e.g. no gpt- prefix) → LiteLLM
|
||||||
llm2 = LLM(model="openai/custom-finetune-model", is_litellm=False)
|
llm2 = LLM(model="openai/custom-finetune-model", is_litellm=False)
|
||||||
assert llm2.is_litellm is True
|
assert llm2.is_litellm is True
|
||||||
assert llm2.model == "openai/custom-finetune-model"
|
assert llm2.model == "openai/custom-finetune-model"
|
||||||
|
assert llm2.provider == "openai"
|
||||||
|
|
||||||
# Test anthropic/ prefix with non-Anthropic model → LiteLLM
|
# Test anthropic/ prefix with non-Anthropic model → LiteLLM
|
||||||
llm3 = LLM(model="anthropic/gpt-4o", is_litellm=False)
|
llm3 = LLM(model="anthropic/gpt-4o", is_litellm=False)
|
||||||
assert llm3.is_litellm is True
|
assert llm3.is_litellm is True
|
||||||
assert llm3.model == "anthropic/gpt-4o"
|
assert llm3.model == "anthropic/gpt-4o"
|
||||||
|
assert llm3.provider == "anthropic"
|
||||||
|
|
||||||
|
|
||||||
def test_prefixed_models_with_valid_patterns_use_native_sdk():
|
def test_prefixed_models_with_valid_patterns_use_native_sdk():
|
||||||
@@ -893,11 +896,38 @@ def test_prefixed_models_with_non_native_providers_use_litellm():
|
|||||||
llm = LLM(model="groq/llama-3.3-70b", is_litellm=False)
|
llm = LLM(model="groq/llama-3.3-70b", is_litellm=False)
|
||||||
assert llm.is_litellm is True
|
assert llm.is_litellm is True
|
||||||
assert llm.model == "groq/llama-3.3-70b"
|
assert llm.model == "groq/llama-3.3-70b"
|
||||||
|
assert llm.provider == "groq"
|
||||||
|
|
||||||
# Test together/ prefix (not a native provider) → LiteLLM
|
# Test together/ prefix (not a native provider) → LiteLLM
|
||||||
llm2 = LLM(model="together/qwen-2.5-72b", is_litellm=False)
|
llm2 = LLM(model="together/qwen-2.5-72b", is_litellm=False)
|
||||||
assert llm2.is_litellm is True
|
assert llm2.is_litellm is True
|
||||||
assert llm2.model == "together/qwen-2.5-72b"
|
assert llm2.model == "together/qwen-2.5-72b"
|
||||||
|
assert llm2.provider == "together"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("model", "expected_provider"),
|
||||||
|
[
|
||||||
|
("groq/llama-3.3-70b", "groq"),
|
||||||
|
("cohere/command-r", "cohere"),
|
||||||
|
("sambanova/Meta-Llama-3.1-70B-Instruct", "sambanova"),
|
||||||
|
("mistral/mistral-large", "mistral"),
|
||||||
|
("vertex_ai/gemini-1.5-pro", "vertex_ai"),
|
||||||
|
("openai/custom-finetune-model", "openai"),
|
||||||
|
("anthropic/gpt-4o", "anthropic"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_litellm_path_preserves_provider_from_model_prefix(model, expected_provider):
|
||||||
|
llm = LLM(model=model, is_litellm=False)
|
||||||
|
assert llm.is_litellm is True
|
||||||
|
assert llm.provider == expected_provider
|
||||||
|
assert llm.model == model
|
||||||
|
|
||||||
|
|
||||||
|
def test_litellm_keeps_provider_but_formats_multimodal_as_openai_schema():
|
||||||
|
llm = LLM(model="anthropic/claude-3-5-haiku-20241022", is_litellm=True)
|
||||||
|
assert llm.provider == "anthropic"
|
||||||
|
assert llm._multimodal_formatter_name() == "openai"
|
||||||
|
|
||||||
|
|
||||||
def test_unprefixed_models_use_native_sdk():
|
def test_unprefixed_models_use_native_sdk():
|
||||||
|
|||||||
@@ -1004,3 +1004,21 @@ def test_internal_instructor_omits_unset_base_url_and_api_key() -> None:
|
|||||||
InternalInstructor(content="x", model=SimpleModel, llm=mock_llm)
|
InternalInstructor(content="x", model=SimpleModel, llm=mock_llm)
|
||||||
|
|
||||||
mock_from_provider.assert_called_once_with("openai/gpt-4o")
|
mock_from_provider.assert_called_once_with("openai/gpt-4o")
|
||||||
|
|
||||||
|
|
||||||
|
def test_internal_instructor_does_not_double_prefix_qualified_models() -> None:
|
||||||
|
from crewai.utilities.internal_instructor import InternalInstructor
|
||||||
|
|
||||||
|
mock_llm = Mock()
|
||||||
|
mock_llm.is_litellm = False
|
||||||
|
mock_llm.model = "groq/llama-3.3-70b"
|
||||||
|
mock_llm.provider = "groq"
|
||||||
|
mock_llm.base_url = None
|
||||||
|
mock_llm.api_key = None
|
||||||
|
|
||||||
|
with patch("instructor.from_provider") as mock_from_provider:
|
||||||
|
mock_from_provider.return_value = Mock()
|
||||||
|
|
||||||
|
InternalInstructor(content="x", model=SimpleModel, llm=mock_llm)
|
||||||
|
|
||||||
|
mock_from_provider.assert_called_once_with("groq/llama-3.3-70b")
|
||||||
|
|||||||
Reference in New Issue
Block a user