address Greptile reviews

This commit is contained in:
Frank Deng 2026-04-25 18:53:31 -07:00
parent d52f4d1821
commit 3e9cf5d5d1
4 changed files with 110 additions and 9 deletions

View file

@ -261,9 +261,7 @@ class FireworksAIConfig(OpenAIGPTConfig):
return {
"supports_function_calling": True,
"supports_tool_choice": True,
"supports_prompt_caching": True, # https://docs.fireworks.ai/guides/prompt-caching
"supports_pdf_input": True, # via document inlining
"supports_vision": True, # via document inlining
"supports_pdf_input": True,
}
def transform_request(
@ -413,7 +411,14 @@ class FireworksAIConfig(OpenAIGPTConfig):
"(or FIREWORKS_AI_API_KEY / FIREWORKSAI_API_KEY / FIREWORKS_AI_TOKEN)."
)
base_url = "https://api.fireworks.ai"
base_url = (
api_base
or get_secret_str("FIREWORKS_API_BASE")
or "https://api.fireworks.ai"
)
if base_url.endswith("/inference/v1"):
base_url = base_url[: -len("/inference/v1")]
base_url = base_url.rstrip("/")
headers = {"Authorization": f"Bearer {api_key}"}
seen: set = set()
result: List[str] = []

View file

@ -4,6 +4,7 @@ For calculating cost of fireworks ai serverless inference models.
from typing import Tuple
from litellm._logging import verbose_logger
from litellm.constants import (
FIREWORKS_AI_4_B,
FIREWORKS_AI_16_B,
@ -74,7 +75,13 @@ def cost_per_token(model: str, usage: Usage) -> Tuple[float, float]:
return generic_cost_per_token(
model=model, usage=usage, custom_llm_provider="fireworks_ai"
)
except Exception:
except Exception as e:
verbose_logger.debug(
"fireworks_ai cost_per_token: model '%s' not in pricing JSON, "
"falling back to size heuristic: %s",
model,
e,
)
base_model = get_base_model_for_pricing(model_name=model)
return generic_cost_per_token(
model=base_model, usage=usage, custom_llm_provider="fireworks_ai"

View file

@ -94,17 +94,23 @@ def test_supports_reasoning_effort():
def test_get_supported_openai_params_reasoning_effort():
"""Test that reasoning_effort is always included — Fireworks accepts it on all models."""
"""
Test that reasoning_effort is always included in supported params.
Verified against live Fireworks API (Apr 2026): sending reasoning_effort
to non-reasoning models (e.g. llama-v3p3-70b-instruct) returns a
successful response the API accepts and silently ignores the parameter.
Models that do support it (qwen3, deepseek-v3p1/v3p2, glm-5p1) use it
to control reasoning depth. The Fireworks API never returns a 4xx for an
unrecognised reasoning_effort value on unsupported models.
"""
config = FireworksAIConfig()
# reasoning_effort should be present for reasoning models
supported_params = config.get_supported_openai_params(
"fireworks_ai/accounts/fireworks/models/qwen3-8b"
)
assert "reasoning_effort" in supported_params
# reasoning_effort should also be present for non-reasoning models
# (Fireworks API accepts it; unsupported models simply ignore it)
other_params = config.get_supported_openai_params(
"fireworks_ai/accounts/fireworks/models/llama-v3-70b-instruct"
)
@ -349,6 +355,80 @@ def test_get_models_account_id_equals_fireworks_no_duplicate_query():
assert result == ["fireworks_ai/accounts/fireworks/models/deepseek-v3p2"]
@pytest.mark.parametrize(
"api_base, expected_base",
[
(
"https://my-proxy.example.com",
"https://my-proxy.example.com/v1/accounts/fireworks/models",
),
(
"https://my-proxy.example.com/inference/v1",
"https://my-proxy.example.com/v1/accounts/fireworks/models",
),
(
"https://my-proxy.example.com/custom/",
"https://my-proxy.example.com/custom/v1/accounts/fireworks/models",
),
(None, "https://api.fireworks.ai/v1/accounts/fireworks/models"),
],
ids=["plain-base", "inference-v1-stripped", "trailing-slash", "default"],
)
def test_get_models_respects_api_base(api_base, expected_base):
"""get_models should use api_base (or FIREWORKS_API_BASE) instead of hardcoding."""
config = FireworksAIConfig()
mock_response = MagicMock()
mock_response.status_code = 200
mock_response.json.return_value = {
"models": [{"name": "accounts/fireworks/models/llama-v3-70b"}]
}
with (
patch(
"litellm.module_level_client.get", return_value=mock_response
) as mock_get,
patch(
"litellm.llms.fireworks_ai.chat.transformation.get_secret_str",
return_value=None,
),
):
config.get_models(api_key="test-key", api_base=api_base)
called_url = mock_get.call_args.kwargs.get("url") or mock_get.call_args[1].get(
"url", ""
)
assert called_url == expected_base
assert "/v1/v1/" not in called_url
def test_get_models_respects_fireworks_api_base_env():
"""get_models should fall back to FIREWORKS_API_BASE env when no api_base arg."""
config = FireworksAIConfig()
mock_response = MagicMock()
mock_response.status_code = 200
mock_response.json.return_value = {"models": []}
with (
patch(
"litellm.module_level_client.get", return_value=mock_response
) as mock_get,
patch(
"litellm.llms.fireworks_ai.chat.transformation.get_secret_str",
side_effect=lambda key: (
"https://env-proxy.example.com" if key == "FIREWORKS_API_BASE" else None
),
),
):
config.get_models(api_key="test-key")
called_url = mock_get.call_args.kwargs.get("url") or mock_get.call_args[1].get(
"url", ""
)
assert called_url.startswith("https://env-proxy.example.com/")
def test_transform_messages_helper_removes_provider_specific_fields():
"""
Test that _transform_messages_helper removes provider_specific_fields from messages.

View file

@ -85,6 +85,15 @@ class TestValidateEnvironment:
)
assert headers["Authorization"] == "Bearer env-key"
def test_should_not_overwrite_existing_authorization(self, config):
params = GenericLiteLLMParams(api_key="new-key")
headers = config.validate_environment(
headers={"Authorization": "Bearer pre-existing"},
model="accounts/fireworks/models/llama-v3-70b",
litellm_params=params,
)
assert headers["Authorization"] == "Bearer pre-existing"
class TestGetCompleteUrl:
"""Tests for get_complete_url."""