Python: fix(bedrock): report the requested model on non-streaming responses

[Somay kousis (Somay-kousis)]

Motivation & Context

BedrockChatClient sends a per-call options["model"] to Converse, but the non-streaming ChatResponse.model is the client's default model, or None if the client has no default. Converse does not return a modelId, so _process_converse_response always falls back to self.model. Telemetry reads gen_ai.response.model from that value, so traces name the wrong model too. Streaming already reports the request's model.

Description & Review Guide

• What are the major changes? The non-streaming path passes request["modelId"] into _process_converse_response as a new keyword-only model argument, and uses it before self.model. This matches what the streaming path already does with request["modelId"]. Added a test with a stub that returns the real Converse response shape (no modelId), for a client with and without a default model.
• What is the impact of these changes? response.model on non-streaming Bedrock responses now names the model the request went to. No public API change. Existing callers of _process_converse_response(response) and (response, options) keep working.
• What do you want reviewers to focus on? Whether to keep the response.get("modelId") / output.get("modelId") lookups. Converse never returns them, but I left them in to keep the change small.

Related Issue

Fixes #9398

I think this is an obvious fix: the streaming path already reports request["modelId"], and #9139 did the same for Gemini. Happy to wait for ready-for-implementation if you'd rather triage #9398 first.

AI Assistance

• No material AI assistance was used.
• This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

AI assistance details: AI tools helped find the bug and write the code, the test and this description. I reviewed all of it and ran the repro, the bedrock tests, and the lint and typing checks myself.

Contribution Checklist

• The code builds clean without any errors or warnings
• All unit tests pass, and I have added new tests where possible
• The PR follows the Contribution Guidelines
• This PR links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
• This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

[Copilot]

🟢 Approval recommended

The focused, backward-compatible fix covers both reported failure cases with no unresolved findings.

0 open findings

What changed in this PR

Fixes #9398 by making non-streaming Bedrock responses report the requested model, correcting model attribution in telemetry.

Changes:

• Passes the resolved request model into response processing while preserving existing callers.
• Adds regression coverage for per-call overrides with and without a client default model.

File	Description
python/​packages/​bedrock/​tests/​test_bedrock_client.py	Tests model reporting using a realistic Converse response.
python/​packages/​bedrock/​agent_framework_bedrock/​_chat_client.py	Uses the requested model before falling back to the client default.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[Somay kousis (Somay-kousis)]

@microsoft-github-policy-service agree

[Somay kousis (Somay-kousis)]

Eduard van Valkenburg (@eavanvalkenburg) The red Python Tests run is one failure outside this change: packages/foundry_hosting/tests/test_responses.py::TestAgentSessionPersistence::test_cancellation_signal_preempts_stuck_agent_call hit its 1 s wait_for timeout on windows-latest / 3.14. The other failed jobs were cancelled after it, and 3.10 and 3.12 on Windows passed in full. It passes locally on this branch. This PR only touches the bedrock package.

Could you re-run the failed jobs?

[Somay kousis (Somay-kousis)]

Eduard van Valkenburg (@eavanvalkenburg) could you re-run the Python Tests here when you get a chance? The only failure was the foundry_hosting cancellation timeout on Windows 3.14, which this bedrock change doesn't touch. Happy to rebase on main first if you'd rather.

[Bicheng (Kenneth) (BichengWang)]

Reviewed 1610dff922e65fedbb4eab39f69a96720d73898c. I found no correctness blocker in the model-reporting change. Passing the resolved request model into the non-streaming response conversion matches the streaming path, and the new keyword-only argument preserves existing calls to _process_converse_response(response) and (response, options).

Offline validation on macOS arm64 / CPython 3.14.7:

• All 82 Bedrock unit tests passed; one integration test was deselected. The isolated invocation emitted an existing unregistered flaky marker warning.
• Both added regression cases fail with the parent implementation: the response reports the client default or None instead of the requested override. Both pass at this head.
• An additional 10-case harness covered default models, overrides with and without a client default, and None/empty override fallback, in both streaming modes. It checked the request model, response model, text, usage, and gen_ai.response.model attribute extraction. Three direct-call compatibility cases also passed.
• Ruff 0.17.0 lint and format checks passed across the Bedrock source and tests. Cached botocore's Converse response shape confirms that the service does not echo modelId.

A useful follow-up test would retain one end-to-end streaming/non-streaming model-parity case or a telemetry attribute assertion; those paths worked in the additional harness but are not pinned by the new committed cases.

I checked existing reviews and inline feedback before reviewing. I did not run live AWS requests, the full workspace suite, type checks, or other Python versions. Maintainer agreement on #9398 remains a separate issue-first prerequisite.

[Somay kousis (Somay-kousis)]

Bicheng (Kenneth) (@BichengWang) thanks for the review. Added the parity case in c9af528: one stub that answers both Converse and ConverseStream without a modelId, parametrized over stream=False/True, asserting the per-call override is the reported model.

Breaking either path fails only its own case:

• non-streaming fix reverted: [False] fails
• streaming switched to self.model: [True] fails

Also rebased on main. 92 Bedrock unit tests pass, ruff 0.17.0 lint and format clean.