feat: support custom model prefix for AWS #2219
Conversation
WalkthroughThis pull request adds support for a configurable AWS model prefix to accommodate AWS Bedrock's updated cross-region deployment naming scheme (e.g., global, eu, jp, us). It introduces a new AWS secret format accepting a region-specific prefix, updates the AWS client initialization and model ID resolution logic, and extends the UI to support the new format option. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Modal as EditChannelModal
participant Backend as relay-aws.go
participant AWS as AWS Bedrock
Client->>Modal: Select AWS channel config
Modal->>Modal: Choose ak_sk_region_prefix format
Modal->>Backend: Submit config with (ak, sk, region, prefix)
Backend->>Backend: newAwsClient()
Backend->>Backend: Parse 4-param secret
Backend->>Backend: Extract modelPrefix from param
Backend->>Backend: Set hasCustomPrefix = true
Backend->>Backend: Create AWS client with region + credentials
Backend->>Backend: Return (client, modelPrefix, true, nil)
Backend->>Backend: doAwsClientRequest()
Backend->>Backend: Receive modelPrefix & hasCustomPrefix
Backend->>Backend: Store a.ModelPrefix = modelPrefix
alt hasCustomPrefix is true
Backend->>Backend: awsModelId = modelPrefix + "." + awsModelId
else hasCustomPrefix is false
Backend->>Backend: awsModelCrossRegion(awsModelId, regionPrefix, "")
Backend->>Backend: Use region-derived prefix if empty configured
end
Backend->>AWS: Call with adjusted awsModelId
AWS-->>Backend: Response
Backend-->>Client: Success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
relay/channel/aws/relay-aws.go (2)
28-86: Handle edge case: empty prefix in 4-parameter formatWhen users provide a 4-parameter secret with an empty prefix (e.g.,
ak|sk|region|), the code setsmodelPrefix = ""andhasCustomPrefix = true. Line 103 indoAwsClientRequestwould then construct.model-id, which is invalid.Consider adding validation after line 73:
region := awsSecret[2] prefix := awsSecret[3] +if prefix == "" { + return nil, "", false, errors.New("prefix cannot be empty in 4-parameter format") +} client = bedrockruntime.New(bedrockruntime.Options{
181-192: Critical: Cross-region logic may bypass map lookupsThere's a logic flaw that could break existing region mappings. When
hasCustomPrefix = false(2-param/3-param formats),doAwsClientRequestpasses the region-derivedmodelPrefix(e.g., "ap") as theconfiguredPrefixparameter at line 109. The new check at line 183 uses this directly if non-empty, bypassing the map lookup at line 187.This breaks the existing behavior where regions like "ap-southeast-1" should map to "jp" prefix according to issue #1928, since the map
awsRegionCrossModelPrefixMaplikely contains this mapping.The
configuredPrefixparameter should only receive the custom prefix from the 4-parameter format. For 2-param/3-param cases, pass an empty string to ensure map lookup is used:func awsModelCrossRegion(awsModelId, awsRegionPrefix, configuredPrefix string) string { - // 如果配置了prefix,优先使用配置的prefix - if configuredPrefix != "" { - return configuredPrefix + "." + awsModelId - } - // 否则从region推导 modelPrefix, find := awsRegionCrossModelPrefixMap[awsRegionPrefix] if !find { return awsModelId } return modelPrefix + "." + awsModelId }And in
doAwsClientRequestat line 109:- awsModelId = awsModelCrossRegion(awsModelId, awsRegionPrefix, modelPrefix) + awsModelId = awsModelCrossRegion(awsModelId, awsRegionPrefix, "")Since the 4-parameter case already handles custom prefixes at line 103, the
configuredPrefixparameter becomes unused and could be removed entirely from the function signature.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
dto/channel_settings.go(1 hunks)relay/channel/aws/adaptor.go(1 hunks)relay/channel/aws/relay-aws.go(4 hunks)web/src/components/table/channels/modals/EditChannelModal.jsx(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-27T02:15:25.448Z
Learnt from: AAEE86
Repo: QuantumNous/new-api PR: 1658
File: web/src/components/table/channels/modals/EditChannelModal.jsx:555-569
Timestamp: 2025-08-27T02:15:25.448Z
Learning: In EditChannelModal.jsx, the applyModelMapping function transforms the models list by replacing original model names (mapping values) with display names (mapping keys). The database stores this transformed list containing mapped keys. On channel load, data.models contains these mapped display names, making the initialization filter if (data.models.includes(key)) correct.
Applied to files:
web/src/components/table/channels/modals/EditChannelModal.jsx
📚 Learning: 2025-08-27T02:15:25.448Z
Learnt from: AAEE86
Repo: QuantumNous/new-api PR: 1658
File: web/src/components/table/channels/modals/EditChannelModal.jsx:555-569
Timestamp: 2025-08-27T02:15:25.448Z
Learning: In EditChannelModal.jsx, the database stores mapped keys (display names) in the models field after applying model mapping transformations. When loading a channel, data.models contains the mapped keys, not the original model names. The filtering logic if (data.models.includes(key)) in the initialization is correct.
Applied to files:
web/src/components/table/channels/modals/EditChannelModal.jsx
🧬 Code graph analysis (1)
relay/channel/aws/relay-aws.go (3)
service/http_client.go (2)
NewProxyHttpClient(66-134)GetHttpClient(49-51)relay/channel/aws/adaptor.go (1)
Adaptor(28-35)types/error.go (1)
ErrorCodeChannelAwsClientError(56-56)
🔇 Additional comments (6)
relay/channel/aws/adaptor.go (1)
34-34: LGTM!The new
ModelPrefixfield provides a clean way to carry the configured prefix through the request lifecycle. The field is properly exported and documented.dto/channel_settings.go (1)
23-23: LGTM!The new constant follows the existing naming pattern and correctly represents the 4-parameter AWS key format.
web/src/components/table/channels/modals/EditChannelModal.jsx (3)
1497-1500: LGTM!The new AWS key format option is properly integrated into the selector, maintaining consistency with the existing options.
1587-1594: LGTM!The batch mode placeholder correctly reflects the 4-parameter format when
ak_sk_region_prefixis selected.
1792-1794: LGTM!The single key mode placeholder properly handles the new region-prefix format.
relay/channel/aws/relay-aws.go (1)
101-111: ****The review comment's specific details are inaccurate. The actual cross-region mapping shows
"ap" → "apac", not"ap" → "jp"as mentioned. Additionally, no reference to issue #1928 exists in the codebase. The comment inadaptor.golists"jp"as a possible prefix value, but it is not present inawsRegionCrossModelPrefixMap.The current implementation correctly handles 2-param and 3-param formats by deriving the region prefix from the region parameter (e.g.,
"ap-southeast-1"→"ap"), which then participates in the cross-region model mapping logic. The 4-param format uses an explicitly configured prefix. All three formats are handled as designed.Likely an incorrect or invalid review comment.
| a.AwsClient = awsCli | ||
| a.ModelPrefix = modelPrefix | ||
|
|
||
| println(info.UpstreamModelName) |
There was a problem hiding this comment.
Remove debug println statement
This debug statement should be removed before merging to production.
Apply this diff:
-println(info.UpstreamModelName)📝 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.
| println(info.UpstreamModelName) |
🤖 Prompt for AI Agents
In relay/channel/aws/relay-aws.go around line 97 there is a leftover debug
println(info.UpstreamModelName); remove this debug print before merging — either
delete the println call or replace it with an appropriate structured logger call
at the correct log level if the model name must be recorded (e.g., use existing
logger.Debug/Info with context), ensuring no stray stdout prints remain.
1 participant
close #1928 ，支持用户自定义
Summary by CodeRabbit