Repository navigation
fix(ci): fail the cost map guard when a Bedrock regional row drifts from its base row #43363
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
dba2b07
e1f2d35
db52939
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2492,6 +2492,7 @@ | |
| "supports_native_structured_output": false, | ||
| "supports_max_reasoning_effort": true, | ||
| "supports_minimal_reasoning_effort": true, | ||
| "supports_output_config": true, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lines 1245 and 2480 at db52939 both set supports_adaptive_thinking true, so the base row has no supports_ key the jp row lacks
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You’re right — I misread the surrounding rows. At |
||
| "supports_parallel_tool_use_config": true, | ||
| "prompt_cache_min_tokens": 2048, | ||
| "source": "https://aws.amazon.com/bedrock/pricing/" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ci_cd/cost_map_guard.py. The repository requires provider-specific code to stay insidellms/. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 inci_cd. I’m withdrawing this finding.