Conversation
Bedrock models resolved their context window from a hardcoded table
(BEDROCK_CONTEXT_LENGTHS) keyed by longest-substring match. AWS ships
new model versions faster than the table tracks, so a new model like
claude-opus-4-8 (1M-token window) silently matched the older
"anthropic.claude-opus-4" entry and got capped at 200K — wasting 80%
of the available context.
Bedrock exposes the real window nowhere in metadata: get-foundation-model
omits it, Converse usage metrics omit it, CountTokens is unsupported on
several models. The only authoritative source is the ValidationException
raised when a prompt exceeds the window:
"prompt is too long: 1300032 tokens > 1000000 maximum"
Length validation runs before inference, so an oversized request is
rejected immediately and cheaply (no tokens generated, no input
processed). This adds probe_bedrock_context_length(): pad a request just
past a tier, parse the reported maximum, return it. get_bedrock_context_length()
now probes first and falls back to the static table only when the probe
can't run (missing creds, network error, unparseable error). The static
table stays as a safety net.
get_model_context_length() caches the probe result per model+region, so
the network cost is paid once, not every turn. probe=False / empty region
disables probing for offline/display paths — backward compatible with the
single-arg callers.
Verified E2E against live Bedrock (eu-central-1): claude-opus-4-8 resolves
to 1000000. Unit tests cover error parsing, unparseable errors, missing
client, probe-beats-table, and table fallback.
There was a problem hiding this comment.
Pull request overview
This PR fixes a class of AWS Bedrock context-window underreporting bugs by dynamically probing Bedrock’s “prompt is too long” validation error to discover the real maximum context length (instead of relying solely on a stale longest-substring static table).
Changes:
- Add a tiered live probe (
probe_bedrock_context_length) and updateget_bedrock_context_lengthto prefer the probe when a region is available. - Update
get_model_context_length()to use Bedrock probing and cache the discovered value so the network cost is paid once. - Add/extend tests for probe parsing, fallback behavior, and backward compatibility (no-region calls skip probing).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
agent/bedrock_adapter.py |
Introduces tiered live probing and updates Bedrock context-length resolution to probe-first with static-table fallback. |
agent/model_metadata.py |
Updates Bedrock context-length resolution path to probe + cache results to avoid repeated network calls. |
tests/agent/test_bedrock_adapter.py |
Adds tests for probe parsing/fallback/back-compat, and verifies probe is skipped when region is absent. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cache_key_url = base_url or "bedrock://" | ||
| cached = get_cached_context_length(model, cache_key_url) | ||
| if cached is not None: | ||
| return cached | ||
| # Resolve region from the base_url host first, then the standard | ||
| # AWS region chain. An empty region disables probing (table only). | ||
| region = "" | ||
| if base_url: | ||
| _m = re.search(r"bedrock-runtime\.([a-z0-9-]+)\.", base_url) | ||
| if _m: | ||
| region = _m.group(1) | ||
| if not region: | ||
| try: | ||
| region = resolve_bedrock_region() | ||
| except Exception: | ||
| region = "" | ||
| ctx = get_bedrock_context_length(model, region=region, probe=bool(region)) | ||
| if ctx and region: | ||
| # Only persist probe-derived values (region present); a pure | ||
| # table fallback shouldn't poison the cache against a later | ||
| # successful probe. | ||
| save_context_length(model, cache_key_url, ctx) | ||
| return ctx |
| Returns the detected window, or ``None`` if the probe could not run | ||
| (missing credentials, network error, or no parseable limit) so the caller | ||
| can fall back to the static table. |
| def _client_raising(self, message): | ||
| client = MagicMock() | ||
| client.converse.side_effect = Exception(message) | ||
| return client | ||
|
|
| def test_probe_returns_none_when_client_unavailable(self): | ||
| from agent.bedrock_adapter import probe_bedrock_context_length | ||
| with patch("agent.bedrock_adapter._get_bedrock_runtime_client", | ||
| side_effect=RuntimeError("boto3 missing")): | ||
| assert probe_bedrock_context_length("any.model", "eu-central-1") is None | ||
|
|
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing the stale Bedrock context lookup. Current main still routes Bedrock through the static table (agent/model_metadata.py:2180-2196), and that table lacks a specific Opus 4.8 entry (agent/bedrock_adapter.py:1298-1342), so the problem remains relevant.
Problems
agent/bedrock_adapter.py:1410returns as soon as the 1.3M tier is accepted. That makes the 2.2M tier unreachable on success and turns an arbitrary lower bound into the cached context value rather than discovering the actual limit.agent/model_metadata.py:1781-1786persists any nonzero resolver result when a region exists, including the static fallback returned after a failed probe. The existing inline review correctly identifies this cache-poisoning path;cache_key_urlat line 1765 also omits the region whenbase_urlis absent.- The added tests do not cover accepted-tier behavior or model-metadata caching for fallback/probe provenance.
Suggested changes
- Carry probe-success provenance separately from the fallback value; cache only parsed probe limits and key them by resolved region.
- Continue a bounded probe after an accepted tier, or treat an accepted lower bound as non-cacheable.
- Add tests for both behaviors above.
Automated hermes-sweeper review.
| "Bedrock context probe for %s accepted ~%s-token prompt; " | ||
| "window is at least that", model_id, f"{tier_tokens:,}", | ||
| ) | ||
| return tier_tokens |
There was a problem hiding this comment.
Returning on the first accepted tier makes the later 2.2M tier unreachable, so any model above 1.3M is recorded as an arbitrary lower bound and then persisted by the caller. Continue the bounded search or mark an accepted lower bound as non-cacheable rather than treating it as the real window.
|
Merged via PR #68007 — your commit was cherry-picked onto current main with your authorship preserved (rebase merge). This was the standout of the Bedrock cluster: reading the real window from the pre-inference length-validation error is the right long-term direction, and the static table is now demoted to an offline fallback/floor instead of the primary source. One reconciliation on top: the stale-cache invalidation that landed just before this (from #44861) was adjusted to floor semantics so it can never discard your probe-derived values. Thanks! |
Problem
Bedrock models resolve their context window from a hardcoded table (
BEDROCK_CONTEXT_LENGTHS) keyed by longest-substring match. AWS ships new model versions faster than the table tracks, so a new model silently matches an older entry and gets capped.Concrete case:
eu.anthropic.claude-opus-4-8(real window 1,000,000 tokens) matches the olderanthropic.claude-opus-4table entry and gets pinned to 200,000 — wasting 80% of the available context. The table currently tops out atopus-4-6, andmaindoes not containopus-4-8, sohermes updatewould not fix it either. This is a whole class of bug, not a one-off: every future model version hits it until someone hand-edits the table.Why a probe (and not "just add opus-4-8 to the table")
Bedrock exposes the real window nowhere in metadata:
get-foundation-modelomits itConverseusage metrics omit itCountTokensis unsupported on several modelsThe only authoritative source is the
ValidationExceptionraised when a prompt exceeds the window:Length validation runs before inference, so an oversized request is rejected immediately and cheaply — no tokens generated, no input processed. Hardcoding
opus-4-8: 1_000_000would just be a change-detector that goes stale atopus-4-9.Change
probe_bedrock_context_length(model_id, region)— pads a request just past a tier, parses the reportedmaximum, returns it. Tiered to avoid opaqueInternalServerExceptionon wildly oversized payloads.get_bedrock_context_length(model_id, region="", probe=True)— probes first, falls back to the static table when the probe cannot run (missing creds, network error, unparseable error). Backward compatible: single-arg callers still work and skip the network.get_model_context_length()caches the probe result per model+region, so the network cost is paid once, not every turn. Pure table fallbacks are not cached, so a later successful probe can still win.Verification
E2E against live Bedrock (eu-central-1):
Tests (via
scripts/run_tests.sh, CI-parity hermetic env):New tests cover: error parsing, unparseable error to None, missing client to None, probe-beats-table, probe-failure to table fallback, and no-region to table-only (probe not called).