address review: drop explanatory comments, stop pinning vendor slugs in tests

Three points from the review, two of which were right.

The helper docstrings explained behaviour the code already shows, which AGENTS.md
rules out. Removed.

The tests asserted that `claude-opus-5` is in the price map and
`claude-opus-5-20250930` is not. Both are facts we do not own: if Anthropic registers
the dated build, the test breaks without any litellm change. They now use a name that
is ours, and assert the invariant instead, that what the provider reports cannot change
the price of a turn the map cannot quote it by.

Picking that name needed care. An arbitrary string makes `get_model_info` raise, which
a caller upstream already handles, so the test passed with or without the fix and
defended nothing. A `claude-` prefixed name reaches the zero-rate fallback that this
bug rides on, so `claude-test-42161` reproduces it and stays ours.

The third point, that lines 801 and 828 exceed 120 characters, does not hold: they are
98 and 39.
This commit is contained in:
Eduardo Pessin 2026-09-22 13:33:07 +01:00 • committed by eduardopessin
parent c5ed8284bd
commit 00b8e37dfc
2 changed files with 43 additions and 62 deletions

View file

@ -801,21 +801,16 @@ def _cost_map_entry_prices_anything(entry: Mapping[str, object]) -> bool:
def _has_rate(model: str, custom_llm_provider: str | None) -> bool:
"""Whether the price map can quote this name.
``get_model_info`` answers a model it does not know with zero rates rather than
raising, so asking it is not enough to tell "free" from "unknown".
"""
# A registered entry is a real answer even when it prices at zero; an unregistered
# model is not, because `get_model_info` invents zero rates for it rather than
# raising. Hence truthiness here, not `is not None`.
if model in litellm.model_cost:
return True
try:
info: Final = litellm.get_model_info(model=model, custom_llm_provider=custom_llm_provider)
except Exception:
return False
return any(
info.get(key)
for key in ("input_cost_per_token", "output_cost_per_token", "input_cost_per_second")
)
return any(info.get(key) for key in ("input_cost_per_token", "output_cost_per_token", "input_cost_per_second"))
def _priced_provider_response_model(
@ -823,29 +818,15 @@ def _priced_provider_response_model(
completion_response_model: str | None,
custom_llm_provider: str | None,
) -> str | None:
"""The provider's own model name, unless charging by it would bill the turn at zero.
Providers report a build rather than a family: Anthropic's ``message_start`` names
``claude-opus-5-20250930`` while the price map carries ``claude-opus-5``. Preferring
the reported name is right when it is priced — it is the most specific truth about
what served the turn — but when the map has never heard of it the turn is billed
``0.0`` with nothing in the logs to say why, because ``get_model_info`` returns zero
rates for an unknown model instead of raising.
Falling back to the response's own model keeps the previous, working behaviour for
that case: it is the name the deployment resolved to, and it is what an unstreamed
turn — which carries no ``provider_response_model`` at all — is already priced by.
"""
if provider_response_model is None:
return None
if _has_rate(provider_response_model, custom_llm_provider):
return provider_response_model
if completion_response_model is not None and _has_rate(completion_response_model, custom_llm_provider):
return completion_response_model
# Neither is priced: keep the provider's name, so the zero that follows is reported
# against what actually served the turn.
return provider_response_model
def _select_model_name_for_cost_calc(
model: str | None,
completion_response: object | None,

View file

@ -5319,72 +5319,72 @@ def test_completion_cost_is_zero_when_explicit_rates_are_zero(monkeypatch: pytes
)
assert cost == 0.0
def test_dated_provider_slug_does_not_bill_the_turn_at_zero():
"""A provider reporting a build the price map does not carry must not zero the turn.
Anthropic names the dated build in `message_start`, so a streamed turn carries
`provider_response_model="claude-opus-5-20250930"` while the map holds
`claude-opus-5`. `_select_model_name_for_cost_calc` prefers the reported name, and
`get_model_info` answers an unknown model with zero rates rather than raising, so the
turn was billed 0.0 with nothing in the logs to explain it.
Unstreamed turns carry no `provider_response_model` and were unaffected, which is why
this looked like a streaming bug. Regression test for #42161.
def test_an_unpriced_provider_slug_does_not_bill_the_turn_at_zero():
"""Regression test for #42161.
`_select_model_name_for_cost_calc` prefers `provider_response_model` over
`response.model`, and `get_model_info` answers a model it has never heard of with
zero rates instead of raising, so a turn reporting an unregistered slug was billed
0.0 with nothing in the logs to explain it. Anthropic hits this by naming the dated
build in `message_start` while the map carries the family.
The invariant: what the provider reports must not change the price of a turn the
price map cannot quote it by.
"""
from litellm.cost_calculator import completion_cost
from litellm.types.utils import Choices, Message, ModelResponse, Usage
unpriced = "claude-test-42161"
assert unpriced not in litellm.model_cost
def _response(reported: str | None) -> ModelResponse:
response = ModelResponse(
model="anthropic/claude-opus-5",
choices=[Choices(message=Message(content="x"))],
)
response.usage = Usage(prompt_tokens=38, completion_tokens=24, total_tokens=62)
response._hidden_params = (
{} if reported is None else {"provider_response_model": reported}
)
response._hidden_params = {} if reported is None else {"provider_response_model": reported}
return response
assert "claude-opus-5" in litellm.model_cost
assert "claude-opus-5-20250930" not in litellm.model_cost
baseline = completion_cost(
completion_response=_response(None), custom_llm_provider="anthropic"
)
baseline = completion_cost(completion_response=_response(None), custom_llm_provider="anthropic")
assert baseline > 0
dated = completion_cost(
completion_response=_response("claude-opus-5-20250930"),
custom_llm_provider="anthropic",
)
assert dated == baseline
reported_unpriced = completion_cost(completion_response=_response(unpriced), custom_llm_provider="anthropic")
assert reported_unpriced == baseline
def test_a_priced_provider_slug_still_wins_over_the_response_model():
def test_a_priced_provider_slug_still_decides_the_rate():
"""The fallback must not cost the reported name its precedence.
When the provider names something the map *does* price, that is the most specific
When the provider names something the map does price, that is the most specific
truth about what served the turn and it still decides the rate.
"""
from litellm.cost_calculator import completion_cost
from litellm.types.utils import Choices, Message, ModelResponse, Usage
priced = "test-model-priced-42161"
litellm.register_model(
model_cost={
priced: {
"input_cost_per_token": 1e-05,
"output_cost_per_token": 2e-05,
"litellm_provider": "anthropic",
"mode": "chat",
}
}
)
def _response(reported: str | None, model: str) -> ModelResponse:
response = ModelResponse(
model=model, choices=[Choices(message=Message(content="x"))]
)
response = ModelResponse(model=model, choices=[Choices(message=Message(content="x"))])
response.usage = Usage(prompt_tokens=38, completion_tokens=24, total_tokens=62)
response._hidden_params = (
{} if reported is None else {"provider_response_model": reported}
)
response._hidden_params = {} if reported is None else {"provider_response_model": reported}
return response
reported_haiku = completion_cost(
completion_response=_response("claude-haiku-4-5", "anthropic/claude-opus-5"),
reported = completion_cost(
completion_response=_response(priced, "anthropic/claude-opus-5"),
custom_llm_provider="anthropic",
)
haiku_directly = completion_cost(
completion_response=_response(None, "anthropic/claude-haiku-4-5"),
custom_llm_provider="anthropic",
)
assert reported_haiku == haiku_directly
expected = 38 * 1e-05 + 24 * 2e-05
assert reported == pytest.approx(expected)