fix(pricing): resolve model prices by longest prefix, not first match - #198
Open
thegoodengineer wants to merge 2 commits into
Open
fix(pricing): resolve model prices by longest prefix, not first match#198thegoodengineer wants to merge 2 commits into
thegoodengineer wants to merge 2 commits into
Conversation
Pricing tables are keyed by model family, but callers pass concrete ids like gpt-4o-mini-2024-07-18. Four lookups resolved those by scanning the table and taking the first prefix hit, so the winner depended on dict insertion order and a cheap variant was priced as the expensive family it extends: - PriceBook.get() gpt-4o-mini-* -> gpt-4o (16.7x) - LiteLLMCostProvider._fallback_cost() gpt-4o-mini-* -> gpt-4 (120x) - CostCalculator fallback rates gpt-4o-mini -> gpt-4o (16.7x) - OpenRouterProvider._calculate_cost() o1-mini-* -> o1 (5x) This mostly hit the small models used as drafters, so it inflated exactly the side of the cascade that is supposed to be cheap. With a gpt-4o-mini-2024-07-18 drafter against a gpt-4o-2024-08-06 verifier, both ids collapsed onto gpt-4o and reported 0% savings for a cascade that actually saves 94%. Match the longest key instead, which makes resolution independent of table order. providers/openai.py and providers/anthropic.py already do this; the shared helper is factored out so the remaining tables use one implementation. Behaviour is unchanged for exact matches, for models with no prefix hit, and for the anthropic and litellm override tables, whose keys were already ordered longest-first.
Each of the four pricing tables is checked against the family a pinned id should resolve to, plus the cascade-savings case that motivated the fix. Also covers the paths that must not change: unknown models still fall back to their table defaults, and the CostCalculator fallback is pinned against _openai_model_pricing so the two OpenAI tables cannot drift apart. 16 of these fail on the previous first-match lookup.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Description
Model pricing tables in this repo are keyed by model family (
gpt-4o), but callers pass concrete model ids (gpt-4o-mini-2024-07-18). Four lookups resolved the concrete id by scanning the table and returning the first prefix hit. Because dicts preserve insertion order, whichever family happened to be declared first won, and a cheap variant got billed at the rate of the more expensive family whose name it extends.gpt-4o-ministarts withgpt-4o, so it matchedgpt-4o. In the LiteLLM fallback table it matchedgpt-4and picked up GPT-4 rates.Affected lookups:
PriceBook.get()gpt-4o-mini-2024-07-18gpt-4oLiteLLMCostProvider._fallback_cost()gpt-4o-mini-2024-07-18gpt-4CostCalculator._estimate_fallback_cost()gpt-4o-minigpt-4oOpenRouterProvider._calculate_cost()openai/o1-mini-2024-09-12openai/o1gpt-5-miniandgpt-5-nanocollapse ontogpt-5the same way (5x and 25x).Why this one stings: the mispriced models are the small ones, and small models are what a cascade drafts with.
gpt-4o-minialone appears in five of the built-in presets as a cheap-tier model. So the error lands on exactly the side of the cascade that is supposed to be cheap, and it inflates the savings figure the library exists to report.With a
gpt-4o-mini-2024-07-18drafter against agpt-4o-2024-08-06verifier, both ids collapsed ontogpt-4o, so a cascade that really saves 94% reported 0% savings.The fix is to match the longest key rather than the first, which makes resolution independent of table order. This is not a new convention:
providers/openai.pyandproviders/anthropic.pyalready do exactly this (max(matches, key=len)), andintegrations/langchain/models.pysorts longest-first for the same reason. I pulled that logic into one small helper so the remaining four tables stop hand-rolling it.🔗 Related Issues
🔄 Type of Change
🧪 Testing
New file
tests/test_pricing_prefix_matching.py, 23 tests. 16 of them fail onmainand pass here, covering all four lookups.The tests also pin down the behaviour that must not change:
CostCalculator's OpenAI fallback is asserted against_openai_model_pricing, so the two OpenAI tables can no longer silently drift apartTest cases added
Full suite
1231 passed before this branch, so no regressions and 23 added.
Also run locally with the versions CI pins:
black --check cascadeflow tests examples(black 25.11.0) cleanruff check cascadeflow tests examples(ruff 0.15.0) cleanbandit -r cascadeflow/ -llno medium or high findingsmypy cascadeflow --ignore-missing-importsfails identically onmainand on this branch in my environment (numpy stub needs a newerpython_versionthanpyproject.tomlsets), so it is untouched by this change.How to test
Before:
After:
📋 Checklist
Code Quality
blackto format my coderuffand fixed all linting issuesmypyfor type checkingTesting
Documentation
Dependencies
Breaking Changes
Costs go down for correctly-priced small models, never up. Exact matches, unmatched models, and the Anthropic and LiteLLM override tables are unaffected, since those tables already had their keys ordered longest-first. The change makes that ordering no longer load-bearing.
📊 Performance Impact
Negligible. The helper builds a list of matching keys and takes the longest instead of breaking on the first hit, so it scans the whole table rather than stopping early. These tables hold on the order of ten to twenty entries and are consulted once per response.
🔐 Security Considerations
Notes for the reviewer
One judgement call worth flagging: I added
cascadeflow/pricing/matching.pyrather than repeatingmax(matches, key=len)a fourth, fifth and sixth time. It is deliberately free of cascadeflow imports soproviders/andtelemetry/can use it without an import cycle.I did not touch
providers/openai.py,providers/anthropic.pyorintegrations/langchain/models.py. They are already correct, and rewriting working code felt like scope creep for a bug fix. They could adopt the helper in a follow-up if you want the duplication gone.providers/groq.pyhas the same first-match pattern but no key in its table is a prefix of another, so it is not currently affected. I left it as is and did not want to widen the diff, though it is fragile the next time a model is added.