fix(ptu): require an operator-declared id on a config.yaml reservation - #37794
Conversation
|
@greptileai @cursor review this. Live proxy and real Postgres evidence is in the description, base versus head on the same scenarios. |
|
bugbot run |
Greptile SummaryThe PR gives config-declared PTU reservations stable operator-provided identities so credential rotation does not create duplicate billing identities
Confidence Score: 5/5The PR appears safe to merge No blocking failure remains
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/ptu_pricing.py | Adds PTU identity validation and support for YAML date objects in reservation bounds |
| litellm/router.py | Captures operator-declared IDs before generated-ID assignment and rejects missing or duplicate PTU identities during registration |
| litellm/proxy/management_endpoints/model_management_endpoints.py | Clarifies that database-backed deployments retain their stable primary-key identity behavior |
| tests/test_litellm/litellm_core_utils/test_ptu_pricing.py | Covers identity requirements, duplicate detection, zero-like IDs, and YAML date parsing |
| tests/test_litellm/test_router_model_cost_isolation.py | Exercises config registration behavior for declared, missing, duplicate, and generated PTU identities |
Reviews (6): Last reviewed commit: "fix(ptu): require an operator-declared i..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
a396ad3 to
8cc9368
Compare
|
@greptileai re-review 8cc9368. Fixed the numeric-id duplicate bypass Devin found, with a regression test. SDK point answered inline. |
|
bugbot run |
|
@greptileai the SDK path already zeroed pricing to 0.0 on base with nothing to charge it, so that config served free. Evidence inline. |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 8cc9368. Configure here.
|
That's correct —
So the 0 spend in the screenshot is the expected zeroed-per-token pricing for a PTU reservation when the flat cost is charged out-of-band — not a gap. Acknowledged, and the logic is consistent. |
|
@greptileai you acknowledged the SDK concern and all three threads are resolved. Please re-score the current head 8cc9368. |
8cc9368 to
8beb446
Compare
|
@greptileai re-review 8beb446. Only change is adding match= to three pytest.raises calls, which the test-tree lint requires. |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 8beb446. Configure here.
A config deployment is otherwise keyed by a hash of its resolved litellm_params, so rotating a credential or editing an endpoint mints a second identity and the catch-up bills the reservation again under it. Flat cost is keyed by that id and a written charge is never retracted, so the duplicate is permanent. The id is read before set_model_list mints one, or the rule would inspect the value it is meant to reject. Duplicates are counted once per config entry across the whole file, so the check is order-independent and an organization fan-out cannot collide with itself. The refusal names the id the deployment already uses, since inventing a fresh one starts exactly the second identity this prevents
8beb446 to
5cf37a1
Compare
|
@greptileai re-review 5cf37a1. Gauntlet found an unquoted yaml date bound skipping PTU entirely, and id 0 read as absent. Both fixed. |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5cf37a1. Configure here.
|
@greptileai review latest head |
tin-berri
left a comment
There was a problem hiding this comment.
Companion to #37793, and just as important — this closes a permanent double-billing bug: a config.yaml PTU deployment is otherwise keyed by a hash of its resolved litellm_params, so routine credential rotation (a standard security practice) mints a second identity and the nightly catch-up re-prices the entire reservation window (up to 91 days) under it, with no rerun able to correct it since written charges are never retracted.
The fix requires an operator-declared model_info.id on any config.yaml deployment with actual PTU terms set — correctly scoped via ptu_terms(_model_info) is not None, so it doesn't touch the vast majority of ordinary config.yaml deployments, and doesn't fire at all with the feature flag off. Duplicate-id detection runs once per set_model_list() across all entries, comparing as strings so a YAML-unquoted numeric id still collides correctly with its string form.
Two real edge-case bugs caught and fixed in the same PR: a falsy id like "0" was being treated as absent by the duplicate scanner (now explicitly tested — test_a_falsy_id_is_still_scanned_for_collisions), and an unquoted ptu_effective_to: 2027-01-01 in YAML parses as a Python date rather than str/datetime, which previously discarded the reservation from PTU handling entirely (billing per-token with zero flat cost) — now handled by matching date before falling through to string parsing. Error message is good UX for a breaking change too: points the operator at GET /model/info to find the id their existing charges are already keyed under. Extensive test coverage, CI green. Approved.
BerriAI/litellm#37794 refuses a config-declared PTU deployment that does not carry an id, so the page can no longer present pinning one as advice. Adds the upgrade note, since setting a fresh name rather than the id the deployment already uses strands the charges already written
…PTU page (#968) * docs(proxy): drop the status code from the PTU rejection note Some invalid PTU configurations answer 422 rather than 400, so naming a single code on the page is misleading. The behaviour that matters is that the rejection names the offending field. * docs(proxy): model_info.id is now required for a config.yaml reservation BerriAI/litellm#37794 refuses a config-declared PTU deployment that does not carry an id, so the page can no longer present pinning one as advice. Adds the upgrade note, since setting a fresh name rather than the id the deployment already uses strands the charges already written
TLDR
Problem this solves:
How it solves it:
model_info.idUser Flow
Before: an admin rotates an Azure key on a schedule and the next morning the team is billed twice for one reservation
LITELLM_ENABLE_PTU_COST_ATTRIBUTION=Trueand a provisioned-throughput deployment declared inconfig.yamlwithapi_key: os.environ/AZURE_API_KEYconfig.yamlAfter: the same rotation changes nothing about the bill
idthey choose, for exampleid: azure-ptu-eastus, undermodel_infoidout is told so at startup, and that one deployment does not load while everything else in the file serves normallyRelevant issues
Linear ticket
Refs LIT-5809
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Real Postgres, a real proxy, and a real Gemini call. Gemini stands in for Azure because provisioned capacity cannot be bought here; the fields the feature reads are provider agnostic and what is under test is the cost path.
Before (ff02d5c)
a reservation declared with no id
The reservation loads, and its identity is a hash of its resolved
litellm_paramsthe credential is rotated
api_key, price it againOne reservation, two identities, and the total doubles
After (5cf37a1)
a reservation declared with no id
The rest of the file is unaffected, and
plain-siblingstill servesthe credential is rotated
id: azure-ptu-eastus, then price the window, rotateapi_key, price it againType
🐛 Bug Fix
Changes
generate_model_idhashes the model group together with every resolvedlitellm_paramsvalue, andset_model_listmints that id only whenmodel_info.idis absent.os.environ/references are resolved before the hash, so rotating a secret changes a deployment's identity without the config file changing at all. Flat cost is keyed by that id, and a written charge is not retracted, so the second identity's charges land beside the first permanently.ptu_identity_errorinlitellm_core_utils.ptu_pricingstates the rule, beside the pricing rules the write endpoints and config registration already share. Registration reads what the operator declared before the id is minted, which is whydeclared_idis threaded in rather than read back offmodel_info, where it would always be present and the rule would never fire.A reservation whose window bound was written unquoted, as
ptu_effective_to: 2027-01-01, was loaded as a date rather than a string and failed to parse, which took the deployment out of PTU handling altogether: no id rule, no zeroing, and no flat cost, while the provider still invoiced the reservation hourly. That bound is now read as the day's opening midnight, so the shape is recognised. An id of0is likewise a declared id, where reading it by truthiness had refused it and had also skipped it in the duplicate scan.The rule is scoped to config-declared deployments carrying PTU fields, while the feature is enabled. Database-backed deployments already hold a stable primary key, so
POST /model/new,PATCH /model/{model_id}/updateand the dashboard's Add Model flow are untouched, as is the per-request credential clone.Caveats (if any)
Final Attestation
Note
Medium Risk
Changes how PTU flat-cost identity is assigned and billed. Existing id-less config.yaml reservations will fail to load when the feature is enabled, which is the intended fix but is a breaking config change.
Overview
Config-declared PTU reservations must now carry an operator-owned
model_info.id, so rotating a credential no longer hashes a new identity and double-bills the reservation window.When PTU cost attribution is on, router registration refuses a config.yaml PTU deployment with no id, or with an id shared by another deployment in the same file. The check uses the declared id before the router mints a hash. DB-backed model endpoints are unchanged; they already have a stable primary key.
Unquoted YAML dates like
2027-01-01are parsed as that day’s UTC midnight instead of dropping the deployment out of PTU handling. An id of0is treated as a real id.Reviewed by Cursor Bugbot for commit 5cf37a1. Bugbot is set up for automated code reviews on this repo. Configure here.