Repository navigation
fix(ci): fail the cost map guard when a Bedrock regional row drifts from its base row - #43363
devin-ai-integration[bot] wants to merge 3 commits into
Conversation
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
| "supports_native_structured_output": false, | ||
| "supports_max_reasoning_effort": true, | ||
| "supports_minimal_reasoning_effort": true, | ||
| "supports_output_config": true, |
There was a problem hiding this comment.
Regional capability still missing Adding
supports_output_config does not bring jp.anthropic.claude-opus-4-7 into parity with its base row: the base has supports_adaptive_thinking: true, but the regional row lacks it. The expanded cross-region test will fail, and the new guard will reject the cost map when it runs.
There was a problem hiding this comment.
Checked at e1f2d35: jp.anthropic.claude-opus-4-7 carries supports_adaptive_thinking: true like its base row, the guard exits 0, and both CircleCI parity tests pass
There was a problem hiding this comment.
Lines 1245 and 2480 at db52939 both set supports_adaptive_thinking true, so the base row has no supports_ key the jp row lacks
There was a problem hiding this comment.
You’re right — I misread the surrounding rows. At db52939f755, both the base and jp. rows already have supports_adaptive_thinking: true. With this change adding supports_output_config: true to the regional row, it matches the base row’s supports_* capabilities. The previous finding is invalid; no further change is needed.
| def _bedrock_base_key(key: str, entries: Entries) -> str | None: | ||
| candidate: Final = BEDROCK_COMMITMENT_PREFIX.sub("", key.replace("*/", "")) | ||
| base: Final = get_bedrock_base_model(candidate) | ||
| if base == candidate: | ||
| return None | ||
| base_key: Final = base if base in entries else f"bedrock/{base}" | ||
| return base_key if base_key in entries and base_key != key else None |
There was a problem hiding this comment.
Bedrock logic outside llms The new guard selects Bedrock rows, normalizes their model names, and compares their capabilities in
ci_cd/cost_map_guard.py. The repository requires provider-specific code to stay inside llms/. This also creates a second place to maintain Bedrock model-name handling; the requirement must be satisfied before merging.
Rule Used: What: Avoid writing provider-specific code outside of the llms/ directory. Why: This practice ensures better maintainability and reduces complexity over time. Good: ```python # Handle provider-specific logic within llms/vertex_ai/transformation.py ... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
The model-name derivation delegates to get_bedrock_base_model in litellm/llms/bedrock/common_utils.py. ci_cd only compares cost-map rows, a CI invariant on the JSON, not provider code
There was a problem hiding this comment.
That’s a fair distinction. Since the guard delegates model-name derivation to the canonical get_bedrock_base_model() implementation and only applies the result to a CI invariant over the cost-map JSON, it is not duplicating provider-specific behavior in ci_cd. I’m withdrawing this finding.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…ort stays offline
|
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 db52939. Configure here.
TLDR
Problem this solves:
How it solves it:
jp.anthropic.claude-opus-4-7) is fixed in both JSON filesglobal.,jp.,au.too)User Flow
Before: a cost-map contributor or the price-sync bot opens a PR adding Bedrock rows, the required check passes, the PR merges, and main goes red
nvidia.nemotron-nano-3-30bwithsupports_audio_input: falseand itsus-gov.copy without that keycost map guard passed (human PR, file checks only), so the PR is mergedlocal_testing_part1on main fails: theus-gov.row lacks asupports_*key its base row carriesAfter: the same PR fails the required check before merge, naming the row and key to fix
us-gov.nvidia.nemotron-nano-3-30b lacks supports_audio_input (false) carried by its base row nvidia.nemotron-nano-3-30b; a Bedrock regional or cross-region row mirrors every supports_* key of its base rowus-gov.row and push, the check passes, and the PR mergeslocal_testing_part1on main stays greenRelevant issues
Related: #42254 (merge 00b298a turned main red on 2026-09-21, repaired by #42640) and #42852 (merge 660e674 did the same on 2026-09-24, repaired by #42951)
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/unit/<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
Each run is the exact command the
cost-map-guardworkflow runs on a PR, from the repo root with the two offending merge commits fetched.--head-refis the PR's branch name (alitellm_cost_map_sync_*head gets the bot contract, anything else the human one). Before runs the guard as it is at the merge base, After runs it as it is at this PR's tipBefore (69d2a3c)
The merge of #42254, which turned main red on 2026-09-21
python ci_cd/cost_map_guard.py --base 00b298a36dd^1 --head 00b298a36dd --head-ref litellm-providers/price-sync-aws-bedrockcost map guard passed (human PR, file checks only), exit 0, although 26 regional rows in that merge lacksupports_audio_input,supports_response_schema,supports_vision, orsupports_function_callingthat their base rows carryThe merge of #42852, which turned main red on 2026-09-24
python ci_cd/cost_map_guard.py --base 660e6746e4a^1 --head 660e6746e4a --head-ref devin/mythos-rowscost map guard passed (human PR, file checks only), exit 0, although threeclaude-mythos-previewcross-region rows saysupports_prompt_caching: truewhile their base row saysfalseThis PR's own cost map change
python ci_cd/cost_map_guard.py --base 69d2a3c24f9 --head db52939f755 --head-ref litellm_cost_map_guard_bedrock_paritycost map guard passed (human PR, file checks only), exit 0After (db52939)
The merge of #42254, which turned main red on 2026-09-21
python ci_cd/cost_map_guard.py --base 00b298a36dd^1 --head 00b298a36dd --head-ref litellm-providers/price-sync-aws-bedrockjp.gap):The merge of #42852, which turned main red on 2026-09-24
python ci_cd/cost_map_guard.py --base 660e6746e4a^1 --head 660e6746e4a --head-ref devin/mythos-rowsThis PR's own cost map change
python ci_cd/cost_map_guard.py --base 69d2a3c24f9 --head db52939f755 --head-ref litellm_cost_map_guard_bedrock_paritycost map guard passed (human PR, file checks only), exit 0: with thejp.row fixed, no Bedrock row on this tip drifts from its baseType
🐛 Bug Fix
🚄 Infrastructure
Caveats (if any)
Medium
jp.fix fail the new check until they merge mainLow
litellmfor the Bedrock base-model derivation, about 6 s more per runLITELLM_LOCAL_MODEL_COST_MAPso that import reads the checkout's cost map instead of fetching main's over the network, the way the unit-test workflows already dopull_request_target), so this PR's owncost-map-guardcheck still ran the old guard; the parity check first runs on PRs opened or updated after this mergessupports_*keys its base lacksinvoke/rows are skipped, as in the testsbedrock, a superset of the two CircleCI tests (those split by exact provider and, before this PR, a fixed prefix list); thejp.row is the only extra gap that superset found on mainuv sync --frozenat db52939 instead: it passes on this PR's own change, fails on the merge of chore(prices): sync AWS Bedrock prices: 13 models, 1 new [1 with gaps, enrichment failed: AWS Bedrock, 38 held] #42254 naming the rows, and skips a PR that leaves the cost map untouchedjp.; pricing parity is outside this guardrun-cilabel goes on next Monday; the twolocal_testing_part1parity tests (test_get_model_info_bedrock_cross_region_capability_parityandtest_get_model_info_bedrock_priced_cross_region_profile_has_priced_base) pass locally at db52939 with the tip's cost mapFinal Attestation
The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR
db52939 passes /live-pr-risk