Repository navigation
fix(groq): register and correctly route compound systems - #32470
devin-ai-integration[bot] wants to merge 2 commits into
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
|
Greptile SummaryThis PR fixes Groq's compound system routing by re-prepending the
Confidence Score: 3/5The routing fix is directionally correct, but the implementation hardcodes model names against the team's explicit JSON-driven pattern, and the key regression test may not actually intercept the HTTP call it intends to verify. The transformation logic is simple and the JSON entries look accurate. However,
|
| Filename | Overview |
|---|---|
| litellm/llms/groq/chat/transformation.py | Adds GROQ_COMPOUND_MODELS frozenset and overrides transform_request to re-prepend groq/ for compound model IDs; violates the team rule requiring model-specific flags to live in the JSON cost map rather than be hardcoded in Python. |
| model_prices_and_context_window.json | Adds entries for groq/compound and groq/compound-mini with 0-cost passthrough pricing, 131072 input and 8192 output tokens, and web-search support; looks correct. |
| litellm/model_prices_and_context_window_backup.json | Mirror of the main JSON changes for the two compound Groq models; identical and consistent with the primary file. |
| tests/test_litellm/llms/groq/chat/test_transformation.py | New test file covering model name mapping and JSON registry; the integration test that calls litellm.completion with a patched HTTPHandler instance may not intercept the actual HTTP call, leaving the core regression assertion untested. |
Reviews (1): Last reviewed commit: "fix(groq): register and correctly route ..." | Re-trigger Greptile
|
|
||
| from ...openai_like.chat.transformation import OpenAILikeChatConfig | ||
|
|
||
| GROQ_COMPOUND_MODELS = frozenset({"compound", "compound-mini"}) |
There was a problem hiding this comment.
Hardcoded model list violates team rule on model-specific flags
GROQ_COMPOUND_MODELS is a hardcoded frozenset in the codebase. The team's explicit rule is to put model-specific flags in model_prices_and_context_window.json and read them via get_model_info, so that adding a new model (e.g. a future groq/compound-pro) works without a LiteLLM code update. The JSON entries for these models are already added in this PR, so a custom field like "requires_groq_prefix": true could be added there and checked via litellm.get_model_info(f"groq/{model}") in _get_groq_model_name. The current approach means any new Groq compound variant requires both a JSON entry and a code change to stay correct.
Rule Used: What: Do not hardcode model-specific flags in the ... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
|
||
| Groq exposes the compound systems with the "groq/" prefix as part of the | ||
| actual model id, so LiteLLM must send "groq/compound(-mini)" instead of the | ||
| prefix-stripped "compound(-mini)", which Groq rejects with model_not_found. | ||
| """ | ||
| client = HTTPHandler() | ||
| fake_response = httpx.Response( | ||
| status_code=200, | ||
| json={ | ||
| "id": "chatcmpl-1", | ||
| "object": "chat.completion", | ||
| "created": 1, | ||
| "model": expected_api_model, | ||
| "service_tier": "auto", | ||
| "choices": [ | ||
| { | ||
| "index": 0, | ||
| "message": {"role": "assistant", "content": "hi"}, | ||
| "finish_reason": "stop", | ||
| } | ||
| ], | ||
| "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2}, | ||
| }, | ||
| request=httpx.Request("POST", "https://api.groq.com/openai/v1/chat/completions"), | ||
| ) | ||
|
|
||
| with patch.object(client, "post", return_value=fake_response) as mock_post: | ||
| litellm.completion( | ||
| model=requested_model, | ||
| messages=[{"role": "user", "content": "hi"}], | ||
| api_key="fake-key", | ||
| client=client, | ||
| ) | ||
|
|
||
| sent_body = json.loads(mock_post.call_args.kwargs["data"]) | ||
| assert sent_body["model"] == expected_api_model |
There was a problem hiding this comment.
Integration test pattern may not intercept the HTTP call as intended
litellm.completion(..., client=client) where client is an HTTPHandler may not forward that instance to the Groq handler's HTTP call. If litellm constructs its own HTTPHandler internally (as it does for most OpenAI-compatible providers), mock_post is never called, mock_post.call_args is None, and the final line raises AttributeError rather than asserting anything meaningful. Other transformation tests in this directory (e.g. Anthropic) call the config's transform_request method directly to inspect the serialized body, which is a more reliable and direct pattern. Verifying this test actually runs green in CI (the CI checkbox is unchecked in the PR) would confirm or dispel the concern.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Closing as stale: no activity in the last 30 days. Feel free to reopen if this is still relevant. |
Relevant issues
Fixes #32467
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
Screenshots / Proof of Fix
I don't have a Groq API key in this environment, so I couldn't capture a real e2e run against
api.groq.com. The exact commands to reproduce before/after are below; I'll add the captured output once a key is availableBefore (on
litellm_internal_staging), calling either compound system fails because LiteLLM strips thegroq/prefix and sendscompound-mini, which Groq rejectsAfter this change LiteLLM sends the full
groq/compound-miniid (the actual Groq model id, see https://console.groq.com/docs/compound) and the two systems are registered in the model cost map solitellm.model_cost/get_model_inforesolve themType
🐛 Bug Fix
Changes
Groq exposes its compound systems (
groq/compoundandgroq/compound-mini) with thegroq/prefix as part of the actual model id. LiteLLM strips the provider prefix before building the request body, so it was sendingcompound/compound-mini, which Groq rejects withmodel_not_found.GroqChatConfig.transform_requestnow re-adds the prefix for these two ids only, leaving every other Groq model (e.g.llama-3.3-70b-versatile,openai/gpt-oss-120b) untouchedBoth systems are also added to
model_prices_and_context_window.jsonand the backup map (context window 131072, max output 8192, chat mode). Groq bills compound as passthrough to the underlying models and server-side tools rather than a fixed per-token rate, soinput_cost_per_token/output_cost_per_tokenare set to 0Tests in
tests/test_litellm/llms/groq/chat/test_transformation.pyassert the registry entries exist with the right metadata and, as the core regression, that a completion forgroq/compound(-mini)putsgroq/compound(-mini)in the request body sent to Groq while a normal model likegroq/llama-3.3-70b-versatileis unchangedLink to Devin session: https://app.devin.ai/sessions/01226ae22f3d438a879200d39ae42875