fix(bedrock): resolve context length via static table before custom-endpoint probe - #14721
Closed
AndreKurait wants to merge 1 commit into
Closed
fix(bedrock): resolve context length via static table before custom-endpoint probe#14721AndreKurait wants to merge 1 commit into
AndreKurait wants to merge 1 commit into
Conversation
…ndpoint probe
## Problem
`get_model_context_length()` in `agent/model_metadata.py` had a resolution
order bug that caused every Bedrock model to fall back to the 128K default
context length instead of reaching the static Bedrock table (200K for
Claude, etc.).
The root cause: `bedrock-runtime.<region>.amazonaws.com` is not listed in
`_URL_TO_PROVIDER`, so `_is_known_provider_base_url()` returned False.
The resolution order then ran the custom-endpoint probe (step 2) *before*
the Bedrock branch (step 4b), which:
1. Treated Bedrock as a custom endpoint (via `_is_custom_endpoint`).
2. Called `fetch_endpoint_model_metadata()` → `GET /models` on the
bedrock-runtime URL (Bedrock doesn't serve this shape).
3. Fell through to `return DEFAULT_FALLBACK_CONTEXT` (128K) at the
"probe-down" branch — never reaching the Bedrock static table.
Result: users on Bedrock saw 128K context for Claude models that
actually support 200K on Bedrock, causing premature auto-compression.
## Fix
Promote the Bedrock branch from step 4b to step 1b, so it runs *before*
the custom-endpoint probe at step 2. The static table in
`bedrock_adapter.py::get_bedrock_context_length()` is the authoritative
source for Bedrock (the ListFoundationModels API doesn't expose context
window sizes), so there's no reason to probe `/models` first.
The original step 4b is replaced with a one-line breadcrumb comment
pointing to the new location, to make the resolution-order docstring
accurate.
## Changes
- `agent/model_metadata.py`
- Add step 1b: Bedrock static-table branch (unchanged predicate, moved).
- Remove dead step 4b block, replace with breadcrumb comment.
- Update resolution-order docstring to include step 1b.
- `tests/agent/test_model_metadata.py`
- New `TestBedrockContextResolution` class (3 tests):
- `test_bedrock_provider_returns_static_table_before_probe`:
confirms `provider="bedrock"` hits the static table and does NOT
call `fetch_endpoint_model_metadata` (regression guard).
- `test_bedrock_url_without_provider_hint`: confirms the
`bedrock-runtime.*.amazonaws.com` host match works without an
explicit `provider=` hint.
- `test_non_bedrock_url_still_probes`: confirms the probe still
fires for genuinely-custom endpoints (no over-reach).
## Testing
pytest tests/agent/test_model_metadata.py -q
# 83 passed in 1.95s (3 new + 80 existing)
## Risk
Very low.
- Predicate is identical to the original step 4b — no behaviour change
for non-Bedrock paths.
- Original step 4b was dead code for the user-facing case (always hit
the 128K fallback first), so removing it cannot regress behaviour.
- Bedrock path now short-circuits before any network I/O — faster too.
- `ImportError` fall-through preserved so users without `boto3`
installed are unaffected.
## Related
- This is a prerequisite for accurate context-window accounting on
Bedrock — the fix for NousResearch#14710 (stale-connection client eviction)
depends on correct context sizing to know when to compress.
Signed-off-by: Andre Kurait <andrekurait@gmail.com>
Contributor
|
Thanks for the fix, @AndreKurait! This has been merged to Automated hermes-sweeper review.
|
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.
What does this PR do?
Fixes a resolution-order bug in
get_model_context_length()that caused every AWS Bedrock model to report the 128K default-fallback context length instead of the correct value from the static Bedrock table (200K for Claude, etc.).The predicate for Bedrock detection is unchanged — only the position of that branch in the resolution order moves from step 4b to step 1b, so it runs before the custom-endpoint probe.
Why?
bedrock-runtime.<region>.amazonaws.comis not in_URL_TO_PROVIDER, so_is_known_provider_base_url()returns False for Bedrock URLs. The original resolution order then:_is_custom_endpoint→ True).fetch_endpoint_model_metadata()→GET /modelson the Bedrock URL (Bedrock doesn't serve this shape).return DEFAULT_FALLBACK_CONTEXT(128K) at the "probe-down" branch on line 1109.Result: users on Bedrock saw 128K context for Claude Sonnet/Opus 4.x (which actually support 200K on Bedrock), triggering premature auto-compression and reducing usable context.
Changes
agent/model_metadata.pytests/agent/test_model_metadata.pyTestBedrockContextResolutionclass (3 regression tests):provider="bedrock"returns the static-table value and does not callfetch_endpoint_model_metadata(assert-not-called is the regression guard — the bug would have called it).base_url="https://bedrock-runtime.us-west-2.amazonaws.com"works without an explicitproviderhint.Which type of PR is this?
How has this been tested?
Also manually verified with
provider="bedrock"on Claude Opus 4.7 — returns 200000 rather than 128000.Risk
Very low:
ImportErrorfall-through preserved for users withoutboto3installed.Checklist
Related work
Complements #14710 (stale-connection client eviction on Bedrock) — correct context sizing is a prerequisite for the compression logic triggered by the error paths in that PR.