Skip to content

[Test Fix] fix gov pricing tests - #25022

Merged
ishaan-berri merged 4 commits into
mainfrom
litellm_fix_gov_pricing_tests
Apr 2, 2026
Merged

[Test Fix] fix gov pricing tests#25022
ishaan-berri merged 4 commits into
mainfrom
litellm_fix_gov_pricing_tests

Conversation

@clfhhc

@clfhhc clfhhc commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

Relevant issues

test_govcloud_completion_cost_calculation
test_govcloud_completion_with_cost_tracking
test_govcloud_cost_per_token_with_region
test_govcloud_model_cost_properties
test_govcloud_model_pricing_verification
test_govcloud_models_in_model_cost

mypy_linting

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have Added testing in the tests/test_litellm/ directory, Adding at least 1 test is a hard requirement - see details
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible, it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

CI (LiteLLM team)

CI status guideline:

  • 50-55 passing tests: main is stable with minor issues.
  • 45-49 passing tests: acceptable but needs attention
  • <= 40 passing tests: unstable; be careful with your merges and assess the risk.
  • Branch creation CI run
    Link:

  • CI run for the last commit
    Link:

  • Merge / cherry-pick CI run
    Links:

Type

🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test

Changes

@vercel

vercel Bot commented Apr 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Apr 2, 2026 10:53pm

Request Review

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@codspeed-hq

codspeed-hq Bot commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 16 untouched benchmarks


Comparing litellm_fix_gov_pricing_tests (797c747) with main (06df8ed)

Open in CodSpeed

@greptile-apps

greptile-apps Bot commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes failing govcloud pricing tests by: (1) adding None-safe guards (or 0.0) throughout cost calculators to handle model entries whose pricing fields exist in the JSON but have a null value; (2) adding new bedrock/us-gov-east-1/anthropic.claude-haiku-4-5-20251001-v1:0 and bedrock/us-gov-west-1/anthropic.claude-haiku-4-5-20251001-v1:0 pricing entries; (3) correcting govcloud Claude Sonnet 4.5 pricing (3.3e-06→3.6e-06 input, 1.65e-05→1.8e-05 output); and (4) aligning govcloud tests to use anthropic.* base model IDs rather than us.* inference-profile IDs, which matches actual Bedrock GovCloud API behavior.

Key changes:

  • litellm/cost_calculator.py, router.py, llms/azure_ai/cost_calculator.py, llms/vertex_ai/cost_calculator.py, litellm_core_utils/llm_request_utils.py: defensive or 0.0 / truthiness guards preventing TypeError when a model's pricing field is None
  • model_prices_and_context_window.json + backup: two new govcloud Haiku 4.5 entries; corrected Sonnet 4.5 govcloud pricing
  • tests/llm_translation/test_bedrock_govcloud.py: model IDs corrected to anthropic.* for GovCloud, region_name removed from base-model cost calls
  • tests/logging_callback_tests/langfuse_expected_request_body/completion_with_bedrock_call.json: totalCost changed from 6.599999999999999e-05 to 6e-05 — the old value was consistent with us.* inference-profile pricing while the new value matches the cheaper base anthropic.* pricing, warranting verification

Confidence Score: 4/5

Safe to merge pending clarification of the totalCost change in the Langfuse fixture, which may indicate cost tracking underreports for us.* inference-profile Bedrock models.

The core govcloud fixes are well-reasoned and the None-guard changes are correct. One P1 finding remains: the Langfuse fixture update changes the expected total cost to a value consistent with cheaper base model pricing rather than the us.* inference-profile pricing declared in the JSON, which contradicts an assertion in the govcloud test suite itself.

tests/logging_callback_tests/langfuse_expected_request_body/completion_with_bedrock_call.json — the totalCost value should be verified against the actual bedrock cost-lookup path for us.* inference profiles.

Important Files Changed

Filename Overview
litellm/cost_calculator.py Adds or 0.0 guard for None pricing values to prevent TypeError on None > 0 comparisons.
litellm/litellm_core_utils/llm_request_utils.py Same None-safe guard in the cheapest-model picker; minor formatting inconsistency introduced.
litellm/llms/azure_ai/cost_calculator.py Adds truthiness check before comparing router_flat_cost_per_token > 0, preventing TypeError when value is None.
litellm/llms/vertex_ai/cost_calculator.py Adds or 0.0 fallback when multiplying by model_info["input/output_cost_per_token"] to handle None values.
litellm/router.py Wraps cost comparisons in (... or 0.0) to guard against None; outer is not None check makes the guard on model_info side redundant but harmless.
model_prices_and_context_window.json Adds two new govcloud Haiku 4.5 entries; corrects Sonnet 4.5 govcloud pricing from 3.3e-06/1.65e-05 to 3.6e-06/1.8e-05.
tests/llm_translation/test_bedrock_govcloud.py Fixes test model IDs to use anthropic.* for GovCloud (not us.*); all tests remain properly mocked with no real network calls.
tests/logging_callback_tests/langfuse_expected_request_body/completion_with_bedrock_call.json Expected totalCost changed from 6.6e-05 (consistent with us.* profile pricing) to 6e-05 (consistent with base anthropic.* pricing) — potential pricing lookup regression.

Reviews (3): Last reviewed commit: "fix test get model info" | Re-trigger Greptile

Comment on lines 203 to 216
@@ -215,7 +215,7 @@ def _handle_128k_pricing(
):
completion_cost = completion_tokens * output_cost_per_token_above_128k_tokens

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Silent $0 cost when pricing is None

Both input_cost_per_token and output_cost_per_token are accessed via model_info["..."] (dict key access that raises KeyError on missing keys) but the or 0.0 fallback silently produces a $0 cost when those keys exist with a None value. If a Vertex AI model somehow has input_cost_per_token: null in the pricing JSON, the cost calculation will silently undercharge users instead of surfacing the missing data.

The govcloud models added in this PR all have explicit non-None pricing, so this won't trigger here. Consider raising a more explicit error or logging a warning when the value is None so it's not silently ignored:

_input_cost = model_info["input_cost_per_token"]
if _input_cost is None:
    verbose_logger.warning("input_cost_per_token is None for model_info, defaulting to 0.0")
    _input_cost = 0.0
prompt_cost = prompt_tokens * _input_cost

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sens but I'm not sure what our mental model is

@ishaan-berri ishaan-berri left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@yuneng-berri
yuneng-berri self-requested a review April 2, 2026 21:40
@clfhhc
clfhhc temporarily deployed to integration-redis-postgres April 2, 2026 22:27 — with GitHub Actions Inactive
@clfhhc
clfhhc temporarily deployed to integration-postgres April 2, 2026 22:27 — with GitHub Actions Inactive
@clfhhc
clfhhc temporarily deployed to integration-postgres April 2, 2026 22:27 — with GitHub Actions Inactive
@clfhhc
clfhhc temporarily deployed to integration-postgres April 2, 2026 22:27 — with GitHub Actions Inactive
@clfhhc
clfhhc temporarily deployed to integration-postgres April 2, 2026 22:27 — with GitHub Actions Inactive
@clfhhc
clfhhc force-pushed the litellm_fix_gov_pricing_tests branch from 04e811d to 797c747 Compare April 2, 2026 22:47
@clfhhc
clfhhc temporarily deployed to integration-redis-postgres April 2, 2026 22:47 — with GitHub Actions Inactive
@clfhhc
clfhhc temporarily deployed to integration-postgres April 2, 2026 22:47 — with GitHub Actions Inactive
@clfhhc
clfhhc temporarily deployed to integration-postgres April 2, 2026 22:47 — with GitHub Actions Inactive
@clfhhc
clfhhc temporarily deployed to integration-postgres April 2, 2026 22:47 — with GitHub Actions Inactive
@clfhhc
clfhhc temporarily deployed to integration-postgres April 2, 2026 22:47 — with GitHub Actions Inactive
@ishaan-berri
ishaan-berri merged commit b7ccc5b into main Apr 2, 2026
98 of 114 checks passed
@ishaan-berri
ishaan-berri deleted the litellm_fix_gov_pricing_tests branch April 2, 2026 22:55
"output": 10,
"unit": "TOKENS",
"totalCost": 6.599999999999999e-05
"totalCost": 6e-05

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Expected cost inconsistent with us.* inference-profile pricing

The totalCost changed from 6.599999999999999e-05 to 6e-05. The model logged is bedrock/us.anthropic.claude-haiku-4-5-20251001-v1:0 with 10 input + 10 output tokens.

Pricing cross-check from model_prices_and_context_window.json:

  • us.anthropic.claude-haiku-4-5-20251001-v1:0input: 1.1e-06, output: 5.5e-0610 × 1.1e-06 + 10 × 5.5e-06 = 6.6e-05 (old value — correct for the us.* profile)
  • anthropic.claude-haiku-4-5-20251001-v1:0 (base Bedrock) → input: 1e-06, output: 5e-0610 × 1e-06 + 10 × 5e-06 = 6e-05 (new value — matches base-model pricing, not the us.* profile)

The new fixture documents that bedrock/us.anthropic.* is now being priced via the cheaper base anthropic.* key, underreporting cost by ~9%. This is also internally inconsistent with test_govcloud_completion_cost_calculation, which still asserts us.* profile pricing for the same model string:

expected_base_cost = 10 * 1.1e-06 + 5 * 5.5e-06
assert abs(base_cost - expected_base_cost) < 1e-10

Please confirm which pricing key the bedrock cost calculator resolves for us.* inference-profile model strings and verify this fixture was not regenerated from a regressed code path.

Rule Used: What: Flag any modifications to existing tests and... (source)

fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 2026
* fix pricing tests

* fix mypy

* fix cost expectation since us based model is used now.

* fix test get model info
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants