fix: allow output_config, metadata and unknown fields in Claude/Bedrock requests#2944
fix: allow output_config, metadata and unknown fields in Claude/Bedrock requests#2944G2-star wants to merge 4 commits intoQuantumNous:mainfrom
Conversation
Allow `output_config`, `thinking` and other new fields, tolerate unknown fields, and avoid 400 errors due to extra fields. Changes: 1. relay/helper/valid_request.go - `GetAndValidateClaudeRequest` now uses `common.UnmarshalBodyReusable` for JSON parsing - Purpose: Use fault-tolerant parsing for Claude/Bedrock endpoints to avoid failures from unknown fields 2. dto/claude.go - `Thinking.Type` now includes `omitempty` - Purpose: Avoid serializing empty strings when `type` is not provided 3. relay/channel/aws/dto.go - Added `OutputConfig json.RawMessage` and `Metadata json.RawMessage` to `AwsClaudeRequest` - Purpose: Pass through `output_config` (including `effort` or future extension fields) and `metadata` without strict validation 4. New Tests - relay/helper/valid_request_test.go: Tests for `output_config` (low/high/empty), `thinking`, unknown field `foo` - relay/channel/aws/dto_test.go: Tests for `output_config` pass-through, `thinking`, unknown field filtering
上游代码将 FileMeta.OriginData 重构为 FileMeta.Source (*FileSource) 使用 NewURLFileSource 和 NewBase64FileSource 创建对应的文件来源
- Merged latest changes from upstream/main (68 commits) - Resolved conflict in relay/channel/aws/dto.go (kept Metadata field) - Fixed FileMeta compatibility issue with new FileSource API
WalkthroughRefactors FileSource creation in Claude DTOs, makes Thinking.Type omit when empty, adds a Metadata field to AwsClaudeRequest, removes response compaction handling, switches Claude request body parsing to UnmarshalBodyReusable, and adds tests for output_config, unknown fields, and Thinking parsing. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
No actionable comments were generated in the recent review. 🎉 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@relay/channel/aws/dto_test.go`:
- Around line 13-36: This test relies on implicit defaults causing
buildAwsRequestBody(nil, ...) to avoid calling common.GetRequestBody(c); make
the behavior explicit by populating the
RelayInfo.ChannelMeta.ChannelSetting.PassThroughBodyEnabled field in the
RelayInfo passed to buildAwsRequestBody (set it to false if you intend to
exercise the non-passthrough path), or if you want to test the passthrough
branch set PassThroughBodyEnabled to true and pass a mocked/non-nil gin.Context
and stub common.GetRequestBody; update the test to set
relaycommon.RelayInfo.ChannelMeta.ChannelSetting.PassThroughBodyEnabled
accordingly to avoid nil gin.Context panics when buildAwsRequestBody or
common.GetRequestBody are invoked.
🧹 Nitpick comments (1)
dto/claude.go (1)
246-252: Consider extracting the duplicated FileSource construction into a helper.The same URL-vs-base64 branching logic appears identically in both the system-media block (lines 246-252) and the message-media block (lines 284-290). A small helper (e.g.,
newFileSourceFromClaudeSource(data string, mediaType string) *types.FileSource) would reduce duplication and make future changes less error-prone.The actual fix — passing
media.Source.MediaTypetoNewBase64FileSource— looks correct.♻️ Proposed helper extraction
// Add near the top of the file or in a shared util func newImageFileMeta(data, mediaType string) *types.FileMeta { var src *types.FileSource if strings.HasPrefix(data, "http://") || strings.HasPrefix(data, "https://") { src = types.NewURLFileSource(data) } else { src = types.NewBase64FileSource(data, mediaType) } return &types.FileMeta{FileType: types.FileTypeImage, Source: src} }Then both call sites simplify to:
if data != "" { - var fileSource *types.FileSource - if strings.HasPrefix(data, "http://") || strings.HasPrefix(data, "https://") { - fileSource = types.NewURLFileSource(data) - } else { - fileSource = types.NewBase64FileSource(data, media.Source.MediaType) - } - fileMeta = append(fileMeta, &types.FileMeta{FileType: types.FileTypeImage, Source: fileSource}) + fileMeta = append(fileMeta, newImageFileMeta(data, media.Source.MediaType)) }Also applies to: 284-290
Address CodeRabbit review: make test intent explicit by setting PassThroughBodyEnabled: false instead of relying on zero-value default.
Summary
ShouldBindJSONwithUnmarshalBodyReusableinGetAndValidateClaudeRequestto tolerate unknown JSON fieldsOutputConfigandMetadatafields (json.RawMessage) toAwsClaudeRequestfor transparent passthrough to BedrockomitemptytoThinking.TypeJSON tag to avoid sending empty"type":""FileMetaconstruction to passMediaTypetoNewBase64FileSourceWhy
Claude's API evolves with new fields (
output_configfor effort control,metadata, etc.). Strict validation viaShouldBindJSONrejects requests containing these fields with 400 errors, breaking forward compatibility. AWS Bedrock needs these fields forwarded as-is for features like adaptive thinking.Changed files
relay/helper/valid_request.gocommon.UnmarshalBodyReusableinstead ofShouldBindJSONdto/claude.goThinking.Typeaddomitempty; fixFileMetabase64 source withMediaTyperelay/channel/aws/dto.goOutputConfigandMetadatafields toAwsClaudeRequestrelay/helper/valid_request_test.gorelay/channel/aws/dto_test.goTest plan
relay/helper/valid_request_test.go— output_config, thinking, unknown fields acceptedrelay/channel/aws/dto_test.go— output_config passes through to Bedrock request bodyoutput_configandmetadatavia both Claude and AWS Bedrock channelsSummary by CodeRabbit
New Features
Bug Fixes
Tests