Well-designed transport-level fix. The two-layer defense (admin config + model-name fallback) is clean, context propagation through the chat loop is sound, and test coverage is thorough across both languages.

Severity breakdown: 9 P3, 1 P4, 3 Nit, 1 Note.

The P3s are a Bedrock provider gap, a missing test preservation case, a silent enum validation gap, several comment/doc quality items, and the dual-source model list coupling. Nothing blocks the fix, but the Bedrock gap (CRF-1) deserves a decision: fix in this PR, file a ticket, or document the limitation.

"Request-capture channels, table-driven boundaries, internal+integration layering. Real stone." (Bisky)

site/src/pages/AgentsPage/components/ChatModelAdminPanel/knownModels/anthropic.ts:12-19

P3 [CRF-5] This comment explains per-model field divergences to help developers add new models, but it does not mention thinkingDisplay. A developer adding claude-opus-4-9 will check this comment, find no mention of thinkingDisplay, and have to reverse-engineer why two entries carry it and four don't.

Two reviewers flag this block from opposite directions: Gon notes it restates what the model entries show; Leorio notes it omits the new field. Suggested rewrite:

// Adaptive-thinking models use `reasoningEffort`; legacy models use
// `thinkingBudgetTokens`. Using `effort` on a legacy model produces
// an Anthropic HTTP 400. Opus 4.7+ defaults `thinking.display` to
// omitted, so those entries carry `thinkingDisplay: "summarized"`.

(Leorio P3, Gon P2)

🤖

🤖 This review was automatically generated with Coder Agents.

All 12 R1 findings addressed cleanly. The Bedrock extension, API validation, version parsing, and naming/doc fixes are well-executed. Test density improved to 60%.

CRF-10 (transport boilerplate duplication, P4) closed by unanimous panel vote (7/7). The two transports have divergent patch signatures; extraction cost exceeds duplication cost at two call sites.

Two new findings this round: 1 P3 (enum coupling), 1 Nit (dead code).

"I tried to build a case against this change and could not." (Pariston)

coderd/x/chatd/chatprovider/chatprovider.go:1214

Nit [CRF-16] withAnthropicThinkingDisplayPatches(httpClient) always returns a non-nil *http.Client (creates &http.Client{} when input is nil). The Anthropic branch (line 1181) correctly removed the equivalent if httpClient != nil guard, but this Bedrock branch still wraps the append in a nil check. Dead code.

Fix: remove the if wrapper and always append fantasybedrock.WithHTTPClient(httpClient), matching the Anthropic branch. (Mafuuu)

🤖

🤖 This review was automatically generated with Coder Agents.

CRF-15 (P3, enum coupling) addressed in 15f39e9: exported shared constants from chatprovider and referenced them in the validation switch. Two of three locations are now compile-time coupled; the SDK struct tag remains a string literal (Go language constraint). Fix looks correct.

CRF-16 (Nit, dead nil check on Bedrock branch at chatprovider.go:1214) has no author response and no code change. withAnthropicThinkingDisplayPatches always returns non-nil, so the if httpClient != nil guard is dead code. The Anthropic branch already removed the equivalent check.

Further panel review is blocked until CRF-16 is addressed: fix the dead code, or respond explaining why it should stay.

🤖 This review was automatically generated with Coder Agents.

All 16 findings resolved across 4 rounds (14 fixed, 1 closed by panel vote, 1 dropped).

The fix is well-scoped: transport-level patching at the Anthropic/Bedrock provider boundary, admin-configurable thinking display, version-aware auto-default for Opus 4.7+, API-boundary validation with shared constants, and thorough test coverage (60% test density, 479 lines of test code).

No open findings remain.

🤖 This review was automatically generated with Coder Agents.