Repository navigation
feat(aws-bedrock): region routing with global default - #2344
Conversation
Add `regionConfig` (global/us/eu/apac) on the aws-bedrock provider with a `modelPrefixMap` so the cross-region inference profile prefix is derived from the selected region. `claude-haiku-4-5` opts in via `regions`; default stays global (us-east-1 endpoint + `global.` prefix). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds AWS Bedrock multi-region support (coarse and specific regions including ChangesAWS Bedrock Multi-Region Configuration and Routing
Sequence Diagram (high-level flow) sequenceDiagram
participant UI as Playground / ProviderKey Dialog
participant API as Gateway / Keys API
participant Resolver as getProviderEndpoint
participant Bedrock as AWS Bedrock Runtime
UI->>API: submit key or request model runtime (with region)
API->>Resolver: resolve endpoint for aws-bedrock (region)
Resolver->>Resolver: pick regionBaseUrl, compute modelPrefix from modelPrefixMap/options
Resolver->>Bedrock: request (region-specific hostname + model path with prefix)
Bedrock-->>Resolver: response
Resolver-->>API: resolved runtime URL
API-->>UI: runtime info / success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@apps/api/src/routes/keys-provider.ts`:
- Around line 31-33: The Zod schema for aws_bedrock_region_prefix currently
lists only ["us.", "global.", "eu."] but aws_bedrock_region accepts "apac", so
update both schema objects that define aws_bedrock_region_prefix to include
"apac." in the z.enum array (i.e., add "apac." alongside "us.", "global.", and
"eu.") so explicit APAC prefix overrides pass validation; ensure you modify both
occurrences of the aws_bedrock_region_prefix enum.
In `@packages/db/src/schema.ts`:
- Around line 577-578: The aws_bedrock_region_prefix union
(aws_bedrock_region_prefix) is missing the "apac." variant while
aws_bedrock_region supports "apac"; update the aws_bedrock_region_prefix type to
include "apac." so the explicit prefix override can select APAC (i.e., add
"apac." to the union alongside "us.", "global.", and "eu."). Ensure the change
is applied where aws_bedrock_region_prefix is declared to keep the two types
consistent.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6807339f-ec81-4b64-ae01-e5a48b7367e4
⛔ Files ignored due to path filters (4)
apps/code/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/playground/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/ui/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsee/admin/src/lib/api/v1.d.tsis excluded by!**/v1.d.ts
📒 Files selected for processing (8)
apps/api/src/routes/keys-provider.tsapps/ui/src/components/provider-keys/create-provider-key-dialog.tsxapps/ui/src/components/provider-keys/provider-keys-list.tsxpackages/actions/src/get-provider-endpoint.spec.tspackages/actions/src/get-provider-endpoint.tspackages/db/src/schema.tspackages/models/src/models/anthropic.tspackages/models/src/providers.ts
💤 Files with no reviewable changes (1)
- apps/ui/src/components/provider-keys/create-provider-key-dialog.tsx
| aws_bedrock_region_prefix?: "us." | "global." | "eu."; | ||
| aws_bedrock_region?: "global" | "us" | "eu" | "apac"; |
There was a problem hiding this comment.
Add apac. to aws_bedrock_region_prefix union for consistency.
With aws_bedrock_region now supporting "apac", the explicit override type should also allow "apac."; otherwise APAC cannot be explicitly selected via prefix override even though routing supports it.
Suggested patch
export interface ProviderKeyOptions {
- aws_bedrock_region_prefix?: "us." | "global." | "eu.";
+ aws_bedrock_region_prefix?: "us." | "global." | "eu." | "apac.";
aws_bedrock_region?: "global" | "us" | "eu" | "apac";📝 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.
| aws_bedrock_region_prefix?: "us." | "global." | "eu."; | |
| aws_bedrock_region?: "global" | "us" | "eu" | "apac"; | |
| aws_bedrock_region_prefix?: "us." | "global." | "eu." | "apac."; | |
| aws_bedrock_region?: "global" | "us" | "eu" | "apac"; |
🤖 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 `@packages/db/src/schema.ts` around lines 577 - 578, The
aws_bedrock_region_prefix union (aws_bedrock_region_prefix) is missing the
"apac." variant while aws_bedrock_region supports "apac"; update the
aws_bedrock_region_prefix type to include "apac." so the explicit prefix
override can select APAC (i.e., add "apac." to the union alongside "us.",
"global.", and "eu."). Ensure the change is applied where
aws_bedrock_region_prefix is declared to keep the two types consistent.
There was a problem hiding this comment.
Pull request overview
Adds region-group routing support for AWS Bedrock (global/us/eu/apac) using the existing regionConfig pattern, with global as the default and region-dependent model prefixing for cross-region inference profiles.
Changes:
- Extend provider region configuration to optionally include a per-region model-id prefix map (
modelPrefixMap). - Add AWS Bedrock
regionConfig(endpoint + prefix mapping) and optclaude-haiku-4-5into multi-region availability. - Plumb
aws_bedrock_regionthrough DB/API/UI and add unit tests for Bedrock region routing.
Reviewed changes
Copilot reviewed 8 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/models/src/providers.ts | Extends ProviderRegionConfig with modelPrefixMap; defines Bedrock region routing config. |
| packages/models/src/models/anthropic.ts | Opts the Bedrock mapping for claude-haiku-4-5 into global/us/eu/apac regions. |
| packages/db/src/schema.ts | Adds aws_bedrock_region to provider key options typings. |
| packages/actions/src/get-provider-endpoint.ts | Uses provider regionConfig endpoint map for Bedrock base URL and applies region-derived model prefixing. |
| packages/actions/src/get-provider-endpoint.spec.ts | Adds unit coverage for Bedrock default + per-region routing and prefix override precedence. |
| apps/api/src/routes/keys-provider.ts | Accepts/returns aws_bedrock_region in provider key create/list schemas. |
| apps/ui/src/components/provider-keys/create-provider-key-dialog.tsx | Removes Bedrock-specific prefix selector; uses generic region selector via regionConfig. |
| apps/ui/src/components/provider-keys/provider-keys-list.tsx | Adds display label for aws_bedrock_region and clarifies legacy prefix label. |
| ee/admin/src/lib/api/v1.d.ts | Updates generated API types to include aws_bedrock_region. |
| apps/ui/src/lib/api/v1.d.ts | Updates generated API types to include aws_bedrock_region. |
| apps/playground/src/lib/api/v1.d.ts | Updates generated API types to include aws_bedrock_region. |
| apps/code/src/lib/api/v1.d.ts | Updates generated API types to include aws_bedrock_region. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case "aws-bedrock": | ||
| url = | ||
| regionBaseUrl ?? | ||
| envValueOrDefault( | ||
| "aws-bedrock", | ||
| "baseUrl", | ||
| "https://bedrock-runtime.us-east-1.amazonaws.com", | ||
| ) ?? "https://bedrock-runtime.us-east-1.amazonaws.com"; | ||
| ) ?? | ||
| "https://bedrock-runtime.us-east-1.amazonaws.com"; |
| const prefix = | ||
| providerKeyOptions?.aws_bedrock_region_prefix ?? | ||
| awsRegionPrefix ?? | ||
| getProviderEnvValue("aws-bedrock", "region", configIndex, "global.") ?? |
| @@ -575,6 +575,7 @@ export const masterKey = pgTable( | |||
|
|
|||
| export interface ProviderKeyOptions { | |||
| aws_bedrock_region_prefix?: "us." | "global." | "eu."; | |||
| @@ -29,6 +29,7 @@ const providerKeySchema = z.object({ | |||
| options: z | |||
| .object({ | |||
| aws_bedrock_region_prefix: z.enum(["us.", "global.", "eu."]).optional(), | |||
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a17b15fabb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| regionBaseUrl ?? | ||
| envValueOrDefault( | ||
| "aws-bedrock", | ||
| "baseUrl", | ||
| "https://bedrock-runtime.us-east-1.amazonaws.com", |
There was a problem hiding this comment.
The new regionBaseUrl ?? envValueOrDefault(...) order causes any regional Bedrock request to ignore an explicitly configured Bedrock base URL (for example LLM_AWS_BEDROCK_BASE_URL in credits/hybrid mode). Before this change, that explicit URL always won; now a :eu/:apac request is forced to the hardcoded AWS endpoint, which can bypass required proxies/private endpoints and break otherwise working deployments. Keep explicit base URL config precedence above derived regional endpoints.
Useful? React with 👍 / 👎.
| @@ -69,6 +70,7 @@ const createProviderKeySchema = z.object({ | |||
| options: z | |||
| .object({ | |||
| aws_bedrock_region_prefix: z.enum(["us.", "global.", "eu."]).optional(), | |||
There was a problem hiding this comment.
After adding APAC regional routing (modelPrefixMap.apac = "apac."), the API schema still rejects aws_bedrock_region_prefix: "apac." because the enum only allows "us." | "global." | "eu.". That blocks clients from using the explicit prefix-override path for APAC and makes the override behavior inconsistent with the newly supported region set. Include "apac." in the Bedrock prefix enum (and matching types) to avoid 400s for valid APAC overrides.
Useful? React with 👍 / 👎.
Extend the aws-bedrock regionConfig with eight specific AWS regions (us-east-1/2, us-west-2, eu-central-1, eu-west-1, ap-northeast-1, ap-southeast-1/2). When a specific region is selected the model is invoked with the bare model ID (no inference profile prefix) routed directly to that region's endpoint, giving true single-region residency for models that support direct invocation. Inference- profile groups (global/us/eu/apac) remain the default for models that require cross-region inference. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/actions/src/get-provider-endpoint.spec.ts (1)
496-536: ⚡ Quick winAdd one non-
skipEnvVarsregression test for specific-region no-prefix behaviorThese new tests only exercise BYOK mode. Add a case with env-var fallback enabled and a non-empty Bedrock prefix env configured, then assert a specific region (e.g.
us-east-2) still uses the bare model ID. That directly guards the empty-string short-circuit behavior.🤖 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 `@packages/actions/src/get-provider-endpoint.spec.ts` around lines 496 - 536, Add a regression test to ensure getProviderEndpoint still uses the bare model ID for specific-region routing when env-var fallback (skipEnvVars=false) and a non-empty Bedrock prefix env is set: create a new it(...) case calling getProviderEndpoint with provider "aws-bedrock", skipEnvVars false (or undefined to enable env fallback), a non-empty bedrock prefix env configured, region "us-east-2" (or another specific region), and expect the returned URL to contain the raw model ID (no prefix) and the correct path (converse or converse-stream depending on the EU flag). Locate and mirror the existing test structure around get-provider-endpoint.spec.ts tests that call getProviderEndpoint so the new test targets the same behaviors (model string like "anthropic.claude-...:0", region "us-east-2", and dataResidency flag) to guard the empty-string short-circuit logic.
🤖 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.
Nitpick comments:
In `@packages/actions/src/get-provider-endpoint.spec.ts`:
- Around line 496-536: Add a regression test to ensure getProviderEndpoint still
uses the bare model ID for specific-region routing when env-var fallback
(skipEnvVars=false) and a non-empty Bedrock prefix env is set: create a new
it(...) case calling getProviderEndpoint with provider "aws-bedrock",
skipEnvVars false (or undefined to enable env fallback), a non-empty bedrock
prefix env configured, region "us-east-2" (or another specific region), and
expect the returned URL to contain the raw model ID (no prefix) and the correct
path (converse or converse-stream depending on the EU flag). Locate and mirror
the existing test structure around get-provider-endpoint.spec.ts tests that call
getProviderEndpoint so the new test targets the same behaviors (model string
like "anthropic.claude-...:0", region "us-east-2", and dataResidency flag) to
guard the empty-string short-circuit logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c016daa4-de5d-46c9-a2f7-ae9cf673cb7b
⛔ Files ignored due to path filters (4)
apps/code/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/playground/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/ui/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsee/admin/src/lib/api/v1.d.tsis excluded by!**/v1.d.ts
📒 Files selected for processing (4)
apps/api/src/routes/keys-provider.tspackages/actions/src/get-provider-endpoint.spec.tspackages/db/src/schema.tspackages/models/src/providers.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30ee9558d7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| defaultRegion: "global", | ||
| regions: [ | ||
| // Cross-region inference profile groups (spread inference across the | ||
| // pool — AWS picks the actual region per request). | ||
| { id: "global", label: "Global (default)" }, |
There was a problem hiding this comment.
Adding Bedrock regionConfig here makes :us/:eu/:apac mappings go through the existing region-key gate in apps/gateway/src/chat/chat.ts (filterRegionsByAvailableKeys + hasRegionSpecificEnvKey), which only allows non-default regions when a __{REGION} env var exists. In credits mode (and hybrid without a DB key), deployments that only set LLM_AWS_BEDROCK_API_KEY will have those regions filtered out even though Bedrock keys are global and token resolution already falls back to the base key (getRegionSpecificEnvValue in packages/models/src/provider.ts). This effectively disables the new regional routing for those projects.
Useful? React with 👍 / 👎.
The playground model selector built the URL as `provider/modelName:region` which leaked the provider-specific upstream model id (e.g. `aws-bedrock/anthropic.claude-haiku-4-5-20251001-v1:0:global`). Switch to `provider/model.id:region` so URLs are clean and stable (`aws-bedrock/claude-haiku-4-5:global`) and the parser now resolves the mapping by region instead of by modelName. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3309422c05
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const ProviderIcon = provider ? getProviderIcon(provider.id) : null; | ||
| const entryKey = `${mapping!.providerId}-${model.id}-${mapping!.modelName}`; | ||
| const providerModelValue = `${mapping!.providerId}/${mapping!.region ? mapping!.modelName : model.id}`; | ||
| const providerModelValue = `${mapping!.providerId}/${mapping!.region ? `${model.id}:${mapping!.region}` : model.id}`; |
There was a problem hiding this comment.
Emit mapping.modelName (or update all consumers together) for regional entries. This change switches the value to ${model.id}:${region}, but playground consumers like getSelectedMapping still resolve by exact mapping.modelName; for mappings where model.id !== mapping.modelName (for example Alibaba regional aliases), the selected regional mapping no longer matches and the UI falls back to a non-regional mapping, which can apply the wrong capability/override state for that selection.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/ui/src/components/provider-keys/create-provider-key-dialog.tsx (1)
199-220: ⚡ Quick winConsider simplifying the error extraction logic.
The current nested error extraction works for common cases but could miss messages in edge cases (e.g., when
error.errorexists as an object without amessageproperty, buterror.messagedoes exist). Using optional chaining and nullish coalescing would be simpler and more robust.♻️ Proposed simplification
onError: (error: unknown) => { setIsValidating(false); let description = "Failed to validate the API key. Please check your key and region."; - if (typeof error === "object" && error !== null) { - const err = error as Record<string, unknown>; - const nested = - err.error && typeof err.error === "object" - ? (err.error as Record<string, unknown>) - : err; - if (typeof nested.message === "string") { - description = nested.message; - } - } else if (error instanceof Error) { + if (error instanceof Error) { description = error.message; + } else if (typeof error === "object" && error !== null) { + const err = error as { error?: { message?: string }; message?: string }; + description = err.error?.message ?? err.message ?? description; } toast({ title: "Validation Failed", description, variant: "destructive", }); },This approach:
- Checks
Errorinstances first (most specific)- Uses a more descriptive type for the error object
- Employs optional chaining to safely extract nested or flat messages
- Falls back gracefully through nullish coalescing
🤖 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 `@apps/ui/src/components/provider-keys/create-provider-key-dialog.tsx` around lines 199 - 220, The onError handler's nested error extraction is overly verbose and can miss cases; modify the onError callback (the function using setIsValidating and toast in create-provider-key-dialog.tsx) to first handle Error instances, then use optional chaining and nullish coalescing to pull a message from (error as any)?.error?.message ?? (error as any)?.message ?? a default message, setIsValidating(false) as before, and pass that message into toast({ title: "Validation Failed", description, variant: "destructive" }) so all nested/flat error shapes are handled succinctly.
🤖 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.
Nitpick comments:
In `@apps/ui/src/components/provider-keys/create-provider-key-dialog.tsx`:
- Around line 199-220: The onError handler's nested error extraction is overly
verbose and can miss cases; modify the onError callback (the function using
setIsValidating and toast in create-provider-key-dialog.tsx) to first handle
Error instances, then use optional chaining and nullish coalescing to pull a
message from (error as any)?.error?.message ?? (error as any)?.message ?? a
default message, setIsValidating(false) as before, and pass that message into
toast({ title: "Validation Failed", description, variant: "destructive" }) so
all nested/flat error shapes are handled succinctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 50efa09d-6fa3-4d41-b521-7239330ffc32
⛔ Files ignored due to path filters (4)
apps/code/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/playground/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsapps/ui/src/lib/api/v1.d.tsis excluded by!**/v1.d.tsee/admin/src/lib/api/v1.d.tsis excluded by!**/v1.d.ts
📒 Files selected for processing (3)
apps/ui/src/components/provider-keys/create-provider-key-dialog.tsxpackages/db/src/schema.tspackages/models/src/models/anthropic.ts
💤 Files with no reviewable changes (2)
- packages/models/src/models/anthropic.ts
- packages/db/src/schema.ts
Routing previously picked the cheapest among all expanded regional candidates for a provider, which for AWS Bedrock meant `:global`, `:us`, `:eu`, `:apac` were equally eligible and the winner was order-dependent. Add `pinDefaultRegion: true` on aws-bedrock so requests without a `:region` suffix and without a provider-key region override always resolve to `:global` (the cross-region inference profile). Alibaba (no `pinDefaultRegion`) keeps its "route to the cheapest region" behavior. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@apps/gateway/src/chat/chat.ts`:
- Around line 1879-1890: The candidate-region filtering calls to
applyPinnedDefaultRegions currently only pass { requestedRegion } which allows
pinDefaultRegion to collapse providers (e.g., "aws-bedrock" -> "global") without
respecting org provider-key locks; update those calls (the ones wrapping
expandAllProviderRegions(...) and filterRegionsByAvailableKeys(...)) to also
pass the providerLockedRegions map (constructed from the providerKeys already
available in those paths) so applyPinnedDefaultRegions can honor
provider-specific region locks; apply the same change in the rate-limit and
low-uptime fallback blocks where providerKeys are present (also update the
occurrences around applyPinnedDefaultRegions usage at the other noted sites).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 92c09951-4afc-43ac-8d57-14e05610a05b
📒 Files selected for processing (2)
apps/gateway/src/chat/chat.tspackages/models/src/providers.ts
Cost calculation and model resolution stripped the :region suffix
with split(":")[0], which broke for upstream model names that
already contain a colon (e.g. AWS Bedrock's
"anthropic.claude-haiku-4-5-20251001-v1:0"). With a :region suffix
applied, the naive split mangled the base name and the lookup fell
through, leading to a $0 billed cost. Use lastIndexOf so only the
trailing :region is removed.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…uting # Conflicts: # apps/playground/src/components/model-selector.tsx
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f11668abc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| regionConfig: { | ||
| optionsKey: "aws_bedrock_region", | ||
| defaultRegion: "global", | ||
| pinDefaultRegion: true, |
There was a problem hiding this comment.
Adding a provider-level regionConfig with defaultRegion: "global" for AWS Bedrock applies a region lock to all Bedrock models, including mappings that do not define regional variants. In chat.ts, resolveRegionFromProviderKey now resolves Bedrock keys to global by default, but for non-regional mappings resolve-provider-context later looks up providerId + modelName + region and fails to match (because those mappings have region undefined). That causes provider capabilities to be read as missing (for example supportsReasoning becomes false and max-output validation is skipped), so requests to non-regional Bedrock models can silently lose reasoning behavior. Restrict the default region application to mappings/providers that actually expose regional entries, or fallback the capability lookup to the non-regional mapping when no region-specific mapping exists.
Useful? React with 👍 / 👎.
modelName is the provider's internal upstream id (e.g. AWS Bedrock's "anthropic.claude-haiku-4-5-20251001-v1:0") and must never leak into user-facing URLs or lookup logic. Remove the modelName-match fallbacks that crept into the playground model selector during the merge — keep only canonical model.id + region resolution. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31af618cee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| : expandAllProviderRegions( | ||
| modelDef.providers as ProviderModelMapping[], | ||
| ), | ||
| { requestedRegion }, |
There was a problem hiding this comment.
When the request uses auto in API-key or hybrid mode and a Bedrock provider key is locked to a non-default region (for example aws_bedrock_region: "eu"), this path has already loaded the provider keys but calls applyPinnedDefaultRegions without passing any explicit locks. Because Bedrock has pinDefaultRegion, the candidate list is reduced to global before selection, usedRegion becomes global, and the later usedRegion ??= resolveRegionFromProviderKey(providerKey) cannot restore the key's EU/APAC choice. This makes auto-routed Bedrock requests ignore the configured provider-key region; build/pass the provider locked-region map here as in the later non-auto routing path.
Useful? React with 👍 / 👎.
…uting # Conflicts: # apps/gateway/src/lib/costs.ts # apps/playground/src/components/model-selector.tsx # apps/ui/src/components/models/playground-model-selector.tsx
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
- Keep an explicit/env Bedrock base URL above the region-derived endpoint so :region requests don't bypass a configured proxy. - Don't read LLM_AWS_BEDROCK_REGION for the model prefix in BYOK mode. - Add `apac.` to the legacy aws_bedrock_region_prefix enums. - Add `sharedCredentialAcrossRegions` so Bedrock regions aren't gated behind per-region env keys in credits mode (the base key is global). - Only apply a provider key's default region to models that actually expose regional mappings, so non-regional Bedrock models keep their capabilities (reasoning, max-output). - Honor provider-key region locks in the auto, rate-limit, and low-uptime fallback routing paths. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary
Add region-specific routing for AWS Bedrock, modeled on Alibaba's
regionConfigpattern. The four regions correspond to AWS cross-region inference profile groups (global,us,eu,apac) — not individual AWS regions.globalis the default and uses the cross-region inference profile (global.model-ID prefix) routed through the us-east-1 endpoint, which is the cheapest, most-available choice and works out of the box with any existing AWS Bedrock long-term API key.claude-haiku-4-5opts in viaregions: [{ id: "global" }, { id: "us" }, { id: "eu" }, { id: "apac" }]. Other AWS Bedrock models are untouched and continue to use the existing single-endpoint path.GCP / Vertex follows in a separate PR.
Notes
ProviderRegionConfigwith an optionalmodelPrefixMapso providers like Bedrock can have a region-dependent model-ID prefix in addition to a region-dependent endpoint URL.aws_bedrock_region_prefixprovider-key option > region-derived prefix > env var >"global.". Existing keys with the legacy prefix option keep working.aws_bedrock_region).Test plan
get-provider-endpoint.spec.tscovering global default, eu/us/apac routing, streaming variant, and provider-key prefix overridepnpm build(17/17 tasks)pnpm formatclaude-haiku-4-5andclaude-haiku-4-5:euafterpnpm setup🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Tests