fix: bedrock reasoning double emission

[TejasGhatte]

Summary

Fixes a follow-on regression from #5108 where a reasoning key (reasoning_config, thinking, or Nova's reasoningConfig) consumed into Params.Reasoning during Bedrock Converse ingress was also forwarded verbatim via additionalModelRequestFieldPaths. This caused the Bedrock egress to carry two copies of the reasoning field — the re-synthesized thinking plus the passthrough — and Converse rejected the collision with "The additional field thinking/type conflicts with an existing field".

Changes

• In ToBifrostResponsesRequest, when Params.Reasoning has been populated, the consumed reasoning keys (thinking, reasoning_config, reasoningConfig) are stripped from the cloned additionalModelRequestFields before forwarding via additionalModelRequestFieldPaths. Non-reasoning fields like output_config are left in place and continue to be deep-merged on egress.
• Added TestReasoningConfigNoDoubleEmissionOnEgress covering all three reasoning key spellings (Anthropic reasoning_config, Anthropic thinking, and Nova reasoningConfig) to assert that exactly one reasoning key appears on the egress wire and no collision occurs.
• Added four E2E Postman collection cases under a new group pinning the /bedrock converse, /bedrock converse-stream, and native /v1/chat/completions entry points, asserting that Bedrock never returns a field-collision error.

Type of change

• Bug fix

Affected areas

• Core (Go)
• Providers/Integrations

How to test

go test ./core/providers/bedrock/... -run TestReasoningConfigNoDoubleEmission -v
go test ./core/providers/bedrock/... -v

Send a Bedrock Converse request with reasoning_config inside additionalModelRequestFields to the /bedrock/model/<model>/converse route and confirm the response does not contain "conflicts with an existing field" or "Remove thinking/".

Breaking changes

• No

Related issues

Closes #5108

Security considerations

None.

Checklist

• I read docs/contributing/README.md and followed the guidelines
• I added/updated tests where appropriate
• I updated documentation where needed
• I verified builds succeed (Go and UI)
• I verified the CI pipeline passes locally if applicable

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 53 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bdbc9e3f-c351-4179-971c-770641f7843f

📥 Commits

Reviewing files that changed from the base of the PR and between af654ad and d024282.

📒 Files selected for processing (3)

• core/providers/bedrock/bedrock_test.go
• core/providers/bedrock/responses.go
• tests/e2e/api/collections/provider-harness.json

📝 Walkthrough

Walkthrough

Bedrock request conversion now removes consumed reasoning fields from forwarded additional model request fields. Unit and provider harness tests cover Anthropic and Nova reasoning inputs across Converse, streaming, and native chat completion routes.

Changes

Bedrock reasoning deduplication

Layer / File(s)	Summary
Reasoning field filtering and unit coverage core/providers/bedrock/responses.go, core/providers/bedrock/bedrock_test.go	Consumed thinking, reasoning_config, and reasoningConfig fields are removed before forwarding extra parameters; round-trip tests verify canonical reasoning keys for Anthropic and Nova.
Provider harness regression coverage tests/e2e/api/collections/provider-harness.json	Adds Converse, streaming Converse, and native chat completion scenarios asserting successful responses without duplicate reasoning-field conflicts.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: akshaydeo, pratham-mishra04, sammaji

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The linked issue asks to fix OrderedMap-based reasoning extraction, but the PR only strips duplicate reasoning keys on egress.	Add the missing inbound extraction fix for *OrderedMap/OrderedMap reasoning fields so fallback requests preserve extended thinking.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Out of Scope Changes check	✅ Passed	The changes stay within Bedrock reasoning-field handling and related tests, with no unrelated additions.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The title clearly and concisely summarizes the Bedrock reasoning double-emission bug fix.
Description check	✅ Passed	The description follows the template well and includes the key summary, changes, test steps, impact, related issue, security, and checklist.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 07-20-fix_bedrock_reasoning_double_emission

---

_{Comment @coderabbitai help to get the list of available commands.}

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[TejasGhatte]

• feat: show server fallback model in logs #5391
• fix: bedrock reasoning double emission #5388 👈 (View in Graphite)
• fix: bill for serving fallback model in anthropic #5387
• fix: anthropic fallbacks and refusal #5367 : 1 other dependent PR (#5373 )
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

[greptile-apps]

Confidence Score: 5/5

This looks safe to merge.

• No blocking issues found in the changed code.

Important Files Changed

Filename	Overview
core/providers/bedrock/responses.go	Filters the three consumed reasoning aliases from cloned passthrough fields before Bedrock egress.
core/providers/bedrock/bedrock_test.go	Adds round-trip tests for Anthropic and Nova reasoning field spellings.
tests/e2e/api/collections/provider-harness.json	Adds Bedrock route coverage for reasoning-field collision errors.

_{Reviews (4): Last reviewed commit: "fix: bedrock reasoning double emission" | Re-trigger Greptile}

[coderabbitai]

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 `@core/providers/bedrock/bedrock_test.go`:
- Around line 6917-6931: Update the reasoning-key assertions in the test case
around tc.wantKey to collect all three possible keys—thinking, reasoning_config,
and reasoningConfig—and assert that the emitted key list contains exactly and
only tc.wantKey. Remove the incomplete pairwise checks while preserving the
existing per-provider expected-key coverage.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: afcae487-e71c-482c-8414-b22791722152

📥 Commits

Reviewing files that changed from the base of the PR and between 6733b11 and af654ad.

📒 Files selected for processing (3)

• core/providers/bedrock/bedrock_test.go
• core/providers/bedrock/responses.go
• tests/e2e/api/collections/provider-harness.json

🚧 Files skipped from review as they are similar to previous changes (2)

• tests/e2e/api/collections/provider-harness.json
• core/providers/bedrock/responses.go

[akshaydeo]

Merge activity

• Jul 20, 5:36 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Jul 20, 5:41 PM UTC: Graphite rebased this pull request as part of a merge.
• Jul 20, 5:42 PM UTC: @akshaydeo merged this pull request with Graphite.

The base branch was changed.