fix(pricing): resolve price collisions by provider rank, not map order - #12
Conversation
Upstream feeds list a model once per place you can buy it. LiteLLM carries 64 GLM entries, six of which collapse onto the canonical ID glm-4.6 with a 5.6x spread: zai 0.60, novita 0.55, vercel 0.45, openrouter 0.40, cerebras 2.25. Canonical() discards the namespace, so vet() kept whichever entry the map happened to visit last. That made every cost llmgate reported unreproducible. Five runs against the same feed gave claude-sonnet-4-5-20250929 at 3.30, 3.30, 3.60, 3.60 and 3.00. Worse for us, glm-4.6 — which vectorless calls on Zhipu's own API — resolved to a reseller rate on every observed run and to Zhipu's actual 0.60/2.20 on none of them, understating spend by 18-25% per call. Enabling UseRemote made the number worse than leaving it off. Collisions now resolve by how authoritative the source is: the vendor's own API, then its global cloud endpoint, then regional, then GovCloud, then resellers. Equal rank falls back to the lexicographically smaller ID, which carries no meaning beyond making the result independent of iteration order. Whoever calls a model's own API — the common case — now gets the rate they are billed. Two further defects surfaced while testing this: Canonical() stripped the date suffix before the version suffix, so "-v1" blocked the date regex and claude-sonnet-4-5-20250929-v1:0 keyed separately from claude-sonnet-4-5-20250929. The dated bucket then held only regional and GovCloud entries, quietly billing their 10-20% premium. Version now comes off first. The drift check ran after the collapse and failed the whole snapshot on one bad row. LiteLLM ships wandb/zai-org/GLM-4.5 at 55000/200000, an upstream units error that lands on glm-4.5; had it won its collision, every other model in the feed would have been discarded with it. Rows are now vetted individually and dropped, with a report through OnError, and only an implausible fraction of the feed condemns the snapshot. Finally, a winning entry that omits cache rates no longer erases the embedded ones — zai/glm-4.6 carries no cache-write rate, and falling through to the family multiplier priced cached reads at 0.30 against the 0.11 z.ai charges. Verified against the live LiteLLM feed: five consecutive fresh-cache runs now resolve glm-4.6, glm-4.5 and glm-4.5-air to Zhipu's own rates, identically, with both poisoned rows dropped and 1070 models retained.
📝 WalkthroughWalkthroughThe pricing pipeline adds GLM classification, improves Bedrock model-ID normalization, ranks canonical pricing collisions deterministically, filters implausible remote rates, reports dropped IDs, and restores missing cache-related rates. ChangesPricing pipeline
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant refresh
participant vet
participant better
participant OnError
refresh->>vet: validate and canonicalize remote prices
vet->>better: resolve canonical model collisions
vet->>OnError: report dropped model IDs
vet-->>refresh: install cleaned pricing snapshot
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuideThis PR fixes non-deterministic and often incorrect remote pricing resolution by ranking upstream providers per canonical model, tightening canonicalization of Bedrock/Anthropic IDs, vetting and reporting implausible rates per-row before collapse, and ensuring embedded cache/extra rates survive silent feeds, backed by new collision-focused tests. Sequence diagram for UseRemote vet errors and dropped rate reportingsequenceDiagram
actor Caller
participant UseRemote
participant Source as src
participant vet
participant OnError as cfg.OnError
participant Store as remote.Store
Caller->>UseRemote: UseRemote(ctx, cfg)
loop periodic refresh
UseRemote->>src: Fetch prices
src-->>UseRemote: raw map[string]Price
UseRemote->>vet: vet(raw)
vet-->>UseRemote: cleaned, dropped, err
alt err != nil
UseRemote->>OnError: OnError(src.Name(), err)
else err == nil
opt len(dropped) > 0 and cfg.OnError != nil
UseRemote->>UseRemote: sortedSample(dropped, 5)
UseRemote->>OnError: OnError(src.Name(), fmt.Errorf(dropped summary))
end
UseRemote->>Store: remote.Store(&snapshot{prices: cleaned, asOf: time.Now(), source: src.Name()})
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
pricing/collision_test.go (1)
156-158: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBuilding the poisoned feed by slicing
glmFeedis fragile.
glmFeed[1:]depends onglmFeedstarting with{and on the template supplying the opening brace and the trailing comma. Any reformatting ofglmFeedbreaks the test with a JSON parse error rather than a clear failure.Define the poisoned row as a separate constant and merge the two objects, or store the fixture as a
map[string]anyand marshal it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pricing/collision_test.go` around lines 156 - 158, Replace the fragile glmFeed[1:] concatenation in the collision test with a separately defined poisoned row and explicitly merge it with the existing feed object before serialization. Keep the resulting fixture valid JSON while removing assumptions about glmFeed’s opening brace and formatting.pricing/remote.go (1)
216-231: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRows with non-positive rates are dropped without any report.
The loop skips rows where
InputPerMTok <= 0orOutputPerMTok <= 0, and whereCanonical(id) == "". These rows never reachdropped, soOnErrornever mentions them. A schema change that zeroes every rate produces only the generic "none usable" error.
Canonical(id)is also computed twice per row, and a third time in the collapse loop. Compute it once.♻️ Proposed refactor
for id, p := range raw { - if p.InputPerMTok <= 0 || p.OutputPerMTok <= 0 { - continue - } - if Canonical(id) == "" { + key := Canonical(id) + if key == "" { continue } - if known, ok := defaultPrices[Canonical(id)]; ok { + if p.InputPerMTok <= 0 || p.OutputPerMTok <= 0 { + dropped = append(dropped, id) + continue + } + if known, ok := defaultPrices[key]; ok {If free-tier rows legitimately carry zero rates, keep the silent skip and count them separately instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pricing/remote.go` around lines 216 - 231, Update the raw pricing loop around Canonical and the rate validation so every discarded row is reported through the existing dropped/OnError flow, while preserving silent handling for legitimate free-tier zero-rate rows by counting them separately if applicable. Compute Canonical(id) once per row, reuse it for validation, defaultPrices lookup, and the later collapse logic, and ensure the resulting reporting still produces the generic none-usable error only when appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pricing/collision_test.go`:
- Around line 72-75: Update the collision assertions around the glm-4.6 check
and the corresponding assertions near lines 145-147 and 189-195 to compare
per-million rates using a small floating-point tolerance instead of exact
equality. Preserve the existing expected values and error messages while
applying the same tolerance-based check consistently to both input and output
rates.
In `@pricing/provider.go`:
- Around line 105-140: Update rankOf to evaluate the model segment (last) before
applying regional ranking, classifying region-pinned Bedrock IDs such as
bedrock/us... as vendor-cloud entries. Restrict regionMarkers matching to the
appropriate namespace portion so incidental eu or us-east text in a model
namespace cannot promote a reseller to regional rank, while preserving
first-party and reseller classification.
In `@pricing/remote.go`:
- Around line 237-244: Update the corruption check in vet to apply an absolute
minimum dropped-row threshold before evaluating the 10% ratio, so a single bad
entry in feeds smaller than ten does not reject the snapshot. Extend
TestPoisonedEntryDoesNotCondemnSnapshot to assert that any reported error
identifies the poisoned entry, ensuring the test exercises per-row drop handling
rather than full-snapshot rejection.
---
Nitpick comments:
In `@pricing/collision_test.go`:
- Around line 156-158: Replace the fragile glmFeed[1:] concatenation in the
collision test with a separately defined poisoned row and explicitly merge it
with the existing feed object before serialization. Keep the resulting fixture
valid JSON while removing assumptions about glmFeed’s opening brace and
formatting.
In `@pricing/remote.go`:
- Around line 216-231: Update the raw pricing loop around Canonical and the rate
validation so every discarded row is reported through the existing
dropped/OnError flow, while preserving silent handling for legitimate free-tier
zero-rate rows by counting them separately if applicable. Compute Canonical(id)
once per row, reuse it for validation, defaultPrices lookup, and the later
collapse logic, and ensure the resulting reporting still produces the generic
none-usable error only when appropriate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 931b22de-fac2-49ee-9040-f6466b82cda9
📒 Files selected for processing (6)
pricing/canonical.gopricing/canonical_test.gopricing/collision_test.gopricing/provider.gopricing/provider_internal_test.gopricing/remote.go
| if p.InputPerMTok != 0.60 || p.OutputPerMTok != 2.20 { | ||
| t.Errorf("glm-4.6 = %.4f/%.4f, want Zhipu's own 0.6000/2.2000 — a reseller rate won the collision", | ||
| p.InputPerMTok, p.OutputPerMTok) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how per-token costs are converted to per-million rates.
rg -n -C4 -e '1e6|1_000_000|1e-6|PerMTok\s*[:=]' pricing/sources.goRepository: hallelx2/llmgate
Length of output: 1641
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- sources relevant sections ---\n'
sed -n '1,320p' pricing/sources.go
printf '\n--- collision_test relevant sections ---\n'
sed -n '1,230p' pricing/collision_test.go
printf '\n--- perMTok usages ---\n'
rg -n 'perMTok\(' pricingRepository: hallelx2/llmgate
Length of output: 245
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat > /tmp/probe_float.js <<'JS'
const inputs = [
['glm-4.6 input', 6.0e-07],
['glm-4.6 output', 2.20e-06],
['Zhipu-like input', 6.000000000000001e-07],
];
function perMTok(perToken) { return perToken * 1_000_000; }
for (const [name, value] of inputs) {
const result = perMTok(value);
console.log(JSON.stringify({ name, input: value, output: result, strictEq: result === 0.60, decimal: result.toFixed(16) }));
}
for (const target of [0.5999999999999999, 0.6000000000000001]) {
console.log(JSON.stringify({ label: target, decimal: target.toFixed(16), equal060: target === 0.60 }));
}
JS
node /tmp/probe_float.jsRepository: hallelx2/llmgate
Length of output: 644
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- sources relevant sections ---'
sed -n '1,320p' pricing/sources.go
printf '%s\n' ''
printf '%s\n' '--- collision_test relevant sections ---'
sed -n '1,230p' pricing/collision_test.go
printf '%s\n' ''
printf '%s\n' '--- perMTok usages ---'
rg -n 'perMTok\(' pricingRepository: hallelx2/llmgate
Length of output: 15672
Compare per-million rates with a small tolerance.
perMTok() multiplies per-token rates by 1_000_000, and decimal-to-float parsing can make values like 0.60e-06 resolve as 0.5999999999999999 or 0.60e-06 plus parsing noise in some feeds. Avoid exact float64 equality in the collision assertions that compare expected per-million rates.
Also applies to lines 145-147 and 189-195.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pricing/collision_test.go` around lines 72 - 75, Update the collision
assertions around the glm-4.6 check and the corresponding assertions near lines
145-147 and 189-195 to compare per-million rates using a small floating-point
tolerance instead of exact equality. Preserve the existing expected values and
error messages while applying the same tolerance-based check consistently to
both input and output rates.
| namespace, last := splitNamespace(s) | ||
|
|
||
| if namespace == "" { | ||
| // No namespace: judge by the dotted prefixes instead. | ||
| switch { | ||
| case strings.HasPrefix(last, "global."): | ||
| return rankVendorCloud | ||
| case hasAnyPrefix(last, dottedRegionPrefixes): | ||
| return rankRegional | ||
| case hasAnyPrefix(last, vendorPrefixes): | ||
| // "anthropic.claude-sonnet-4-5-v1:0" — a Bedrock ID that | ||
| // happens to carry no region. | ||
| return rankVendorCloud | ||
| default: | ||
| // A bare "claude-sonnet-4-5" or "gpt-4o": the vendor's own | ||
| // catalogue entry, and the best evidence there is. | ||
| return rankFirstParty | ||
| } | ||
| } | ||
|
|
||
| // A namespaced ID. The first segment names who sells it. | ||
| head := namespace | ||
| if before, _, found := strings.Cut(namespace, "/"); found { | ||
| head = before | ||
| } | ||
|
|
||
| if containsAny(namespace, regionMarkers) { | ||
| return rankRegional | ||
| } | ||
| if fam := familyOf(s); slices.Contains(firstPartyNamespaces[fam], head) { | ||
| return rankFirstParty | ||
| } | ||
| if slices.Contains(vendorCloudNamespaces, head) { | ||
| return rankVendorCloud | ||
| } | ||
| return rankReseller |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for namespaced IDs that also carry a dotted region or global prefix in fixtures and feeds.
rg -n --glob '*.go' --glob '*.json' -e '"[a-z_]+/(global|us|eu|apac|au|jp|ca|sa)\.' .
# Show all rank-related tests to confirm the covered ID shapes.
rg -n 'rankOf\(' -C2 .Repository: hallelx2/llmgate
Length of output: 1246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== pricing/provider.go outline =="
ast-grep outline pricing/provider.go || true
echo "== pricing/provider.go relevant lines =="
cat -n pricing/provider.go | sed -n '80,210p'
echo "== pricing/provider_internal_test.go relevant lines =="
cat -n pricing/provider_internal_test.go | sed -n '1,140p'
echo "== feed/fixture namespaced dotted IDs =="
rg -n --glob '*.go' --glob '*.json' --glob '*.yml' --glob '*.yaml' -e '[[:alnum:]-_]+/(global\.)?[[:alnum:]-_]+\.[[:alnum:]-_:\.]+|[[:alnum:]-_]+/(us|eu|apac|au|jp|ca|sa)\.[[:alnum:]-_\.:]+' .Repository: hallelx2/llmgate
Length of output: 15375
🌐 Web query:
LiteLLM model_prices_and_context_window.json Bedrock cross region inference dotted region namespaces examples
💡 Result:
LiteLLM supports Amazon Bedrock cross-region inference by allowing you to use region-prefixed model IDs in your configuration [1][2]. Configuration and Usage To use cross-region inference, you specify the model ID with its region prefix (e.g., us, eu, apac) within the model string [1][3][4]. Example configuration in config.yaml: model_list: - model_name: bedrock-claude-haiku litellm_params: model: bedrock/us.anthropic.claude-3-haiku-20240307-v1:0 aws_region_name: us-east-1 Model Pricing and Context Window Mapping LiteLLM maps these models in model_prices_and_context_window.json [5][6]. Because cross-region inference profiles have specific identifiers, LiteLLM requires explicit entries in this JSON file to accurately track costs and context windows [5][7]. Key points regarding the pricing map: 1. Mapping Requirements: LiteLLM resolves costs by looking for keys that match the full prefixed model name (e.g., bedrock/us.anthropic.claude-3-5-haiku-20240307-v1:0) [5][7]. 2. Resolution Logic: If a specific prefixed entry is missing, LiteLLM may fail to calculate costs correctly or report $0 pricing [8][6][7]. While some modern models have these regional "twins" pre-mapped, users may occasionally encounter issues if a new regional profile is not yet added to the repository [8][6]. 3. Current Status: Most widely used Claude models (e.g., 3-5, 3-7, 4-x series) already include entries for standard regional prefixes like us. and eu. [6]. Troubleshooting Tips If you encounter a "model isn't mapped yet" error or incorrect cost reporting: - Verify the exact model string: Ensure it follows the expected pattern, such as bedrock/{region}.{provider}.{model-id} [5][3]. - Check for Route Overrides: You can explicitly specify the routing path if needed by using bedrock/converse/{model} or bedrock/invoke/{model} [1][2]. - Application Profiles: For advanced use cases involving AWS Application Inference Profiles (which use ARNs), the ARN should generally be provided in the model_id parameter within your litellm_params [9].
Citations:
- 1: https://docs.litellm.ai/docs/providers/bedrock
- 2: https://github.com/BerriAI/litellm/blob/main/docs/my-website/docs/providers/bedrock.md
- 3: [Feature]: Support Bedrock's Cross Region Inference feature BerriAI/litellm#5899
- 4: Add support for Bedrock cross-region inference BerriAI/litellm#5898
- 5: Fix: Bedrock cross-region inference profile cost calculation BerriAI/litellm#14566
- 6: Missing eu./us. regional Bedrock model entries in model_prices_and_context_window.json BerriAI/litellm#24202
- 7: [Feature]: cost for bedrock Cross region inference model isnt mapped BerriAI/litellm#8115
- 8: [Bug]: Bedrock cross-region inference pricing entries in cost map are unreachable due to lookup order BerriAI/litellm#24669
- 9: How can we use Bedrock Application Inference Profiles? BerriAI/litellm#8905
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import urllib.request, json
url = "https://github.com/ghraw/BerriAI/litellm/main/model_prices_and_context_window.json"
data = json.load(urllib.request.urlopen(url, timeout=20))
keys = [k for k in data if isinstance(k, str) and "/" in k.lower()]
dotted = [k for k in keys if "/" in k and k.rsplit("/", 1)[1].lower().startswith(("global.", "us.", "eu.", "apac.", "au.", "jp.", "ca.", "sa."))]
print("namespaced_keys", len(keys))
print("namespaced_dotted", len(dotted))
for k in sorted(dotted)[:100]:
print(k)
PYRepository: hallelx2/llmgate
Length of output: 2440
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== inspect canonical.go for mapping behavior =="
cat -n pricing/canonical.go | sed -n '1,110p'
echo "== inspect remote.go parsing/ranking behavior =="
cat -n pricing/remote.go | sed -n '220,290p'
echo "== deterministic rankOf implementation probe (without running repo code) =="
python3 - <<'PY'
import re
# Mirrors the relevant rankOf logic from pricing/provider.go.
vendor_cloud_namespaces = {"bedrock", "vertex_ai", "azure", "azure_ai"}
first_party_namespaces = {
"zai": ["zai", "z-ai"],
"anthropic": ["anthropic"],
"openai": ["openai"],
"gemini": ["gemini"],
"vertex": [],
}
family_re = re.compile(r'^([a-z_\-]+)[./]', re.I)
region_markers = ["us-east", "us-west", "ap-southeast", "ap-east", "ap-northeast", "me-central", "eu", "us-gov", "usgov", "china"]
dotted_region_prefixes = ["us.", "eu.", "apac.", "au.", "jp.", "ca.", "sa."]
def rank_of(id):
s = id.lower().strip()
if s == "":
return "reseller"
for g in region_markers:
if g in s:
return "gov"
i = s.rfind("/")
if i < 0:
namespace, last = "", s
else:
namespace, last = s[:i], s[i+1:]
if not namespace:
if last.startswith("global."):
return "vendor_cloud"
if any(last.startswith(p) for p in dotted_region_prefixes):
return "regional"
return "vendor_cloud"
head = namespace.split("/", 1)[0]
if any(sub in namespace for sub in region_markers):
return "regional"
fam_match = family_re.search(s)
if fam_match and head in first_party_namespaces.get(fam_match.group(1), []):
return "first_party"
if head in vendor_cloud_namespaces:
return "vendor_cloud"
return "reseller"
cases = [
"bedrock/us-east-1/z-ai.glm-4.6",
"bedrock/us.anthropic.claude-sonnet-4-5-v1:0",
"bedrock/global.anthropic.claude-sonnet-4-5-v1:0",
"reseller/us-east-hosting/glm-4.6",
"some-new-gateway/us.anthropic.claude-sonnet-4-5-v1:0",
]
for c in cases:
print(c, "=>", rank_of(c))
PYRepository: hallelx2/llmgate
Length of output: 7997
Check the model segment before assigning regional rank.
rankOf ranks namespaced IDs by the namespace, so Bedrock region-pinned IDs like bedrock/us.anthropic.claude-sonnet-4-5-v1:0 are treated as vendor-cloud entries. For model collisions, that can let a cheaper global/non-region-ID win unless the better lexicographic source happens to have a cache rate or lower ID. Also restrict regionMarkers checks so a model namespace containing eu or us-east does not make that reseller rank regionally.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pricing/provider.go` around lines 105 - 140, Update rankOf to evaluate the
model segment (last) before applying regional ranking, classifying region-pinned
Bedrock IDs such as bedrock/us... as vendor-cloud entries. Restrict
regionMarkers matching to the appropriate namespace portion so incidental eu or
us-east text in a model namespace cannot promote a reseller to regional rank,
while preserving first-party and reseller classification.
| // A handful of bad rows is upstream noise. A large fraction of them is | ||
| // a units error or a schema change, and adopting it would misreport | ||
| // spend by orders of magnitude — so that still condemns the snapshot. | ||
| if len(dropped) > len(raw)/10 { | ||
| return nil, dropped, fmt.Errorf( | ||
| "%d of %d entries have implausible rates — treating snapshot as corrupt", | ||
| len(dropped), len(raw)) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The corruption threshold rejects small feeds after a single dropped row.
len(raw)/10 uses integer division. If len(raw) < 10, the threshold is 0, so one dropped row condemns the whole snapshot. That contradicts the stated intent that a handful of bad rows is upstream noise.
This also weakens TestPoisonedEntryDoesNotCondemnSnapshot in pricing/collision_test.go. That feed has 9 entries and 1 poisoned row, so vet returns an error and UseRemote installs no snapshot. Lookup then falls back to the embedded table, which already prices glm-4.6 at 0.60, and errs is non-empty because OnError receives the vet error. The test passes without exercising the new per-row drop path.
Add an absolute floor before the ratio applies.
🐛 Proposed fix
- if len(dropped) > len(raw)/10 {
+ // A ratio alone rejects small feeds on a single bad row, so require a
+ // minimum absolute count as well.
+ const minDroppedForCorrupt = 4
+ if len(dropped) >= minDroppedForCorrupt && len(dropped)*10 > len(raw) {
return nil, dropped, fmt.Errorf(
"%d of %d entries have implausible rates — treating snapshot as corrupt",
len(dropped), len(raw))
}After the fix, extend the regression test to assert that the reported error names the dropped ID, so a full-snapshot rejection cannot satisfy it.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // A handful of bad rows is upstream noise. A large fraction of them is | |
| // a units error or a schema change, and adopting it would misreport | |
| // spend by orders of magnitude — so that still condemns the snapshot. | |
| if len(dropped) > len(raw)/10 { | |
| return nil, dropped, fmt.Errorf( | |
| "%d of %d entries have implausible rates — treating snapshot as corrupt", | |
| len(dropped), len(raw)) | |
| } | |
| // A handful of bad rows is upstream noise. A large fraction of them is | |
| // a units error or a schema change, and adopting it would misreport | |
| // spend by orders of magnitude — so that still condemns the snapshot. | |
| // A ratio alone rejects small feeds on a single bad row, so require a | |
| // minimum absolute count as well. | |
| const minDroppedForCorrupt = 4 | |
| if len(dropped) >= minDroppedForCorrupt && len(dropped)*10 > len(raw) { | |
| return nil, dropped, fmt.Errorf( | |
| "%d of %d entries have implausible rates — treating snapshot as corrupt", | |
| len(dropped), len(raw)) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pricing/remote.go` around lines 237 - 244, Update the corruption check in vet
to apply an absolute minimum dropped-row threshold before evaluating the 10%
ratio, so a single bad entry in feeds smaller than ten does not reject the
snapshot. Extend TestPoisonedEntryDoesNotCondemnSnapshot to assert that any
reported error identifies the poisoned entry, ensuring the test exercises
per-row drop handling rather than full-snapshot rejection.
The bug
Upstream price feeds list a model once per place you can buy it. LiteLLM carries 64 GLM entries, and six collapse onto the canonical ID
glm-4.6:zai/glm-4.6together_ai/zai-org/GLM-4.6novita/zai-org/glm-4.6vercel_ai_gateway/zai/glm-4.6openrouter/z-ai/glm-4.6cerebras/zai-glm-4.6Canonical()throws the namespace away, sovet()kept whichever entry the map visited last — a 5.6x spread decided by iteration order.Five runs against the same feed gave
claude-sonnet-4-5-20250929at 3.30, 3.30, 3.60, 3.60, 3.00. Andglm-4.6resolved to a reseller rate on every observed run and to Zhipu's real 0.60/2.20 on none, understating spend 18-25% per call. Turning onUseRemotemade the number worse than leaving it off.The fix
Collisions resolve by how authoritative the source is — vendor's own API > its global cloud > regional > GovCloud > resellers — with a lexicographic tie-break whose only job is to remove the dependence on map order.
Two further defects surfaced while testing:
Canonical()stripped the date before the version suffix, so-v1blocked the date regex andclaude-sonnet-4-5-20250929-v1:0keyed separately fromclaude-sonnet-4-5-20250929. The dated bucket then held only regional and GovCloud entries, quietly billing their 10-20% premium.wandb/zai-org/GLM-4.5at 55000/200000; had it won its collision, all 1,070 other models would have gone with it. Rows are now vetted individually, dropped, and reported viaOnError; only an implausible fraction condemns the snapshot.Also: a winning entry that omits cache rates no longer erases the embedded ones.
zai/glm-4.6carries no cache-write rate, and the family-multiplier fallback priced cached reads at 0.30 against the 0.11 z.ai charges.Verification
Against the live LiteLLM feed, five consecutive fresh-cache runs:
Identical every run. Before this change the same probe gave 0.55/2.20, then 0.45/1.80, then 0.45/1.80.
New tests use the real upstream IDs and rates — the previous fixtures had one entry per canonical key, so no collision was ever constructed and the bug was invisible to every existing test.
TestCollisionIsDeterministicinstalls the same feed 60 times to exercise many map orderings.Note
vet()is unexported; its signature changed to return dropped IDs. No public API change.Closes HAL-563
Summary by Sourcery
Resolve remote pricing collisions by provider precedence instead of map iteration order, and harden vetting and canonicalization to produce stable, accurate rates from noisy upstream feeds.
Bug Fixes:
Enhancements:
Tests:
Summary by CodeRabbit