Fix/aws header override - #3066
Merged
Merged
Conversation
Co-authored-by: G2-star <G2-star@users.noreply.github.com>
Contributor
|
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:
WalkthroughRuntime header override resolution was exposed via a new public function and integrated into the AWS relay request flow; request DTOs for AWS and Vertex channels gained optional Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Incoming Request
participant Handler as AWS Relay Handler
participant Resolver as ResolveHeaderOverride
participant Processor as processHeaderOverride
participant AWS as AWS Bedrock
Client->>Handler: HTTP request with runtime header overrides
Handler->>Resolver: ResolveHeaderOverride(relayInfo, ctx)
Resolver->>Processor: processHeaderOverride(...)
Processor-->>Resolver: resolved header map
Resolver-->>Handler: return resolved overrides
Handler->>Handler: apply overrides to headers/body (e.g., anthropic_beta)
Handler->>AWS: send InvokeModelInput with applied overrides
AWS-->>Handler: response
Handler-->>Client: proxied response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dto/claude.go`:
- Around line 429-432: The Thinking struct currently marks the Type field with
`omitempty`, which allows omission but the Anthropic Claude API requires the
`type` field; remove the `omitempty` tag from Thinking.Type in the Thinking
struct definition (or ensure callers always set a valid "enabled"/"disabled"
value before JSON serialization) so the serialized payload always includes the
required `type` field; update any constructors or code paths that build Thinking
to set a default if necessary.
In `@relay/channel/vertex/dto.go`:
- Line 23: The copyRequest function fails to copy the new Metadata field from
dto.ClaudeRequest into the VertexAIClaudeRequest, so include req.Metadata =
src.Metadata (or equivalent) when building the VertexAIClaudeRequest in
copyRequest; update the copyRequest logic to read src.Metadata (from
dto.ClaudeRequest) and assign it to the target VertexAIClaudeRequest.Metadata so
Vertex AI requests carry over the metadata field.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
dto/claude.gorelay/channel/api_request.gorelay/channel/aws/dto.gorelay/channel/aws/relay-aws.gorelay/channel/aws/relay_aws_test.gorelay/channel/vertex/dto.gorelay/helper/valid_request.go
Comment on lines
429
to
432
| type Thinking struct { | ||
| Type string `json:"type"` | ||
| Type string `json:"type,omitempty"` | ||
| BudgetTokens *int `json:"budget_tokens,omitempty"` | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Anthropic Claude API thinking type field specification documentation
💡 Result:
thinking field (request) — Claude Messages API
To enable extended thinking, include a top-level thinking object in your Messages API request with:
"thinking": { "type": "enabled", "budget_tokens": 2048 }type:"enabled"(to turn on extended thinking)budget_tokens: maximum token budget for thinking; must be at least 1024 and (in the normal/non-interleaved case) less thanmax_tokens; the thinking budget counts toward yourmax_tokenslimit. [1][2]
To disable:
"thinking": { "type": "disabled" }
``` [2]
### `thinking` content blocks (response)
When enabled, the assistant response `content` will include one or more blocks with:
* `type`: `"thinking"`
* `thinking`: the model’s thinking text (what you can display, depending on model/version)
* `signature`: an opaque verification value; Anthropic notes full thinking may be encrypted and returned via `signature` (not meant to be parsed). [1][3]
### Streaming events for thinking
In streaming mode, thinking arrives via:
* `thinking_delta` events (incremental thinking text)
* a `signature_delta` event **just before** `content_block_stop` for that thinking block. [3]
---
**Sources:** Anthropic “Building with extended thinking” docs [1], Anthropic API token counting docs (request schema notes for `thinking`) [2], Anthropic “Streaming Messages” docs (thinking/signature deltas) [3], Anthropic API release notes (signature behavior) [4].
---
</details>
**Do not add `omitempty` to `Thinking.Type` — the `type` field is required by the Anthropic API.**
The Anthropic Claude API documentation explicitly requires the `type` field in thinking objects, with values of either `"enabled"` or `"disabled"`. Using `omitempty` allows this field to be omitted when empty, which violates the API specification and could cause requests to fail. Either ensure `Type` is always set to a valid value before serialization, or remove the `omitempty` tag.
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @dto/claude.go around lines 429 - 432, The Thinking struct currently marks
the Type field with omitempty, which allows omission but the Anthropic Claude
API requires the type field; remove the omitempty tag from Thinking.Type in
the Thinking struct definition (or ensure callers always set a valid
"enabled"/"disabled" value before JSON serialization) so the serialized payload
always includes the required type field; update any constructors or code paths
that build Thinking to set a default if necessary.
</details>
<!-- This is an auto-generated comment by CodeRabbit -->
| ToolChoice any `json:"tool_choice,omitempty"` | ||
| Thinking *dto.Thinking `json:"thinking,omitempty"` | ||
| OutputConfig json.RawMessage `json:"output_config,omitempty"` | ||
| Metadata json.RawMessage `json:"metadata,omitempty"` |
Contributor
There was a problem hiding this comment.
Metadata field is not copied in copyRequest.
The new Metadata field was added to VertexAIClaudeRequest, but copyRequest (lines 26-41) does not copy req.Metadata from the source dto.ClaudeRequest. This means Vertex AI requests will never include the metadata even when the original request contains it.
🐛 Proposed fix to include Metadata in copyRequest
func copyRequest(req *dto.ClaudeRequest, version string) *VertexAIClaudeRequest {
return &VertexAIClaudeRequest{
AnthropicVersion: version,
System: req.System,
Messages: req.Messages,
MaxTokens: req.MaxTokens,
Stream: req.Stream,
Temperature: req.Temperature,
TopP: req.TopP,
TopK: req.TopK,
StopSequences: req.StopSequences,
Tools: req.Tools,
ToolChoice: req.ToolChoice,
Thinking: req.Thinking,
OutputConfig: req.OutputConfig,
+ Metadata: req.Metadata,
}
}📝 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.
Suggested change
| Metadata json.RawMessage `json:"metadata,omitempty"` | |
| func copyRequest(req *dto.ClaudeRequest, version string) *VertexAIClaudeRequest { | |
| return &VertexAIClaudeRequest{ | |
| AnthropicVersion: version, | |
| System: req.System, | |
| Messages: req.Messages, | |
| MaxTokens: req.MaxTokens, | |
| Stream: req.Stream, | |
| Temperature: req.Temperature, | |
| TopP: req.TopP, | |
| TopK: req.TopK, | |
| StopSequences: req.StopSequences, | |
| Tools: req.Tools, | |
| ToolChoice: req.ToolChoice, | |
| Thinking: req.Thinking, | |
| OutputConfig: req.OutputConfig, | |
| Metadata: req.Metadata, | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@relay/channel/vertex/dto.go` at line 23, The copyRequest function fails to
copy the new Metadata field from dto.ClaudeRequest into the
VertexAIClaudeRequest, so include req.Metadata = src.Metadata (or equivalent)
when building the VertexAIClaudeRequest in copyRequest; update the copyRequest
logic to read src.Metadata (from dto.ClaudeRequest) and assign it to the target
VertexAIClaudeRequest.Metadata so Vertex AI requests carry over the metadata
field.
3 tasks
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@service/channel_affinity.go`:
- Around line 439-450: The merge logic for the "operations" key uses the raw
template key k, so differently-cased/whitespace keys (e.g., " Operations ")
won't match existing base entries; normalize the key before reading/writing by
computing normKey := strings.ToLower(strings.TrimSpace(k)) (or constant
"operations") and use normKey when calling extractParamOperations and when
reading/writing out (e.g., check out[normKey], assign out[normKey] = ...); if
the original raw key differs from normKey, remove or replace out[k] to avoid
duplicate keys. Ensure you still call extractParamOperations(v) for the template
value v and preserve the existing append order (tplOps before baseOps).
Comment on lines
+439
to
+450
| if strings.EqualFold(strings.TrimSpace(k), "operations") { | ||
| baseOps, hasBaseOps := extractParamOperations(out[k]) | ||
| tplOps, hasTplOps := extractParamOperations(v) | ||
| if hasTplOps { | ||
| if hasBaseOps { | ||
| out[k] = append(tplOps, baseOps...) | ||
| } else { | ||
| out[k] = tplOps | ||
| } | ||
| continue | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Normalize the target operations key before reading/writing.
Line 440 reads out[k] using the raw template key. If template uses " Operations " and base uses "operations", base ops won’t merge and you can end up with duplicate operation keys.
🔧 Proposed fix
for k, v := range tpl {
if strings.EqualFold(strings.TrimSpace(k), "operations") {
- baseOps, hasBaseOps := extractParamOperations(out[k])
+ opsKey := "operations"
+ for existingKey := range out {
+ if strings.EqualFold(strings.TrimSpace(existingKey), "operations") {
+ opsKey = existingKey
+ break
+ }
+ }
+ baseOps, hasBaseOps := extractParamOperations(out[opsKey])
tplOps, hasTplOps := extractParamOperations(v)
if hasTplOps {
- if hasBaseOps {
- out[k] = append(tplOps, baseOps...)
- } else {
- out[k] = tplOps
- }
+ mergedOps := tplOps
+ if hasBaseOps {
+ mergedOps = append(tplOps, baseOps...)
+ }
+ out[opsKey] = mergedOps
+ if opsKey != k {
+ delete(out, k)
+ }
continue
}
}
out[k] = v
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@service/channel_affinity.go` around lines 439 - 450, The merge logic for the
"operations" key uses the raw template key k, so differently-cased/whitespace
keys (e.g., " Operations ") won't match existing base entries; normalize the key
before reading/writing by computing normKey :=
strings.ToLower(strings.TrimSpace(k)) (or constant "operations") and use normKey
when calling extractParamOperations and when reading/writing out (e.g., check
out[normKey], assign out[normKey] = ...); if the original raw key differs from
normKey, remove or replace out[k] to avoid duplicate keys. Ensure you still call
extractParamOperations(v) for the template value v and preserve the existing
append order (tplOps before baseOps).
hardingCheng
pushed a commit
to hardingCheng/new-api
that referenced
this pull request
Mar 3, 2026
* main: feat: auto fetch upstream models (QuantumNous#2979) feat: add AionUI to chat settings and built-in templates Revert "fix: aws text content blocks must be non-empty" Revert "Fix/aws non empty text" fix: tool responses Return error when model price/ratio unset Merge pull request QuantumNous#3066 from seefs001/fix/aws-header-override fix: handle rate limits and improve error response parsing in video task updates fix: default empty input_json_delta arguments to {} for tool call parsing fix: preserve tool_use on malformed tool arguments to keep tool_result pairing valid fix: aws text content blocks must be non-empty feat: add cc-switch integration and modal for token management fix: preserve explicit zero values in native relay requests fix: enhance migrateTokenModelLimitsToText function to return errors and improve migration checks fix: migrate model_limits column from varchar(1024) to text for existing tables fix: change token model_limits column from varchar(1024) to text # Conflicts: # web/src/i18n/locales/en.json # web/src/i18n/locales/fr.json # web/src/i18n/locales/ja.json # web/src/i18n/locales/ru.json # web/src/i18n/locales/vi.json
This was referenced Mar 5, 2026
ennnnny
pushed a commit
to ennnnny/new-api
that referenced
this pull request
Mar 17, 2026
…ride Fix/aws header override
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
2 participants
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.
fix #2944 #3041
Summary by CodeRabbit
New Features
Tests
Improvements
Chores