fix(anthropic): preserve the provider-qualified response model (#1117) - #1138
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
✅ Deterministic PR hygiene checks passed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1879bd0df6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const preserveAnthropicResponseModel = route.providerName === "anthropic" | ||
| || route.provider.adapter === "anthropic"; |
There was a problem hiding this comment.
Include finalized Anthropic wire routes in preservation
When a client requests opencode-go/minimax-m3 (similarly minimax-m2.5 or minimax-m2.7), the provider initially has the openai-chat adapter, so this predicate is false; line 891 then applies the repository's hard pin and switches the finalized adapter to anthropic. Consequently, parsed.modelId is stripped to minimax-m3 without setting _responseModelId, and JSON/SSE responses still return the bare selector, so round-tripping it loses the opencode-go route—the same failure this change is intended to fix. Include the finalized adapter or pinned-wire result in the preservation predicate and add coverage for a pinned Anthropic route.
Useful? React with 👍 / 👎.
Summary
A request for
anthropic/claude-sonnet-5came back asclaude-sonnet-5.applyFinalRouteRequestNormalizationoverwritesparsed.modelIdwith the bare upstream id so the Anthropic request goes out correctly, but every downstream consumer then builtresponse.modelfrom that mutated value — the Responses bridge, the image loop, and the web-search loop. A client that round-tripsresponse.modelloses the provider routing.The final Codex-facing selector is now retained on
_responseModelIdand used for client-facing output on all three paths, while the upstream request body keeps the bare model. Request logs keep the physical routed model, so observability still shows what was actually called.Scope guard. This is deliberately Anthropic-only:
_responseModelIdis set only when the routed provider is Anthropic, so every other provider leaves it undefined and?? parsed.modelIdyields byte-identical behavior. A regression test pins that — a non-Anthropic routed provider whose public and wire model differ still emits exactly what it emitted before. Response identity for every provider is a contract change, not a bug fix, and this PR does not make it.Closes #1117.
Attribution
The mechanism is @giulioleone097's (Giulio Leone) from #1122 — the
_responseModelIdfield, the 29-lineresponses-model-rewrite.tshelper, the bridge/loop wiring, and the request-log preservation are all their design, credited viaCo-authored-by. They also reported the issue.@Ingwannu independently diagnosed the same defect correctly in #1121; credit for the diagnosis is theirs too.
What I intentionally did not carry over from #1122: the catalog changes (
sync.ts,parsing.ts,provider-fetch.ts,effort.ts,convergence.ts), the hidden bare-selector compatibility rows, and theadapter-resolve.tscaptured-default change. Generated catalog rows with restore/removal semantics carry their own failure modes (user-owned row collisions, restore deleting a generated row) and deserve their own PR rather than riding along with a response-identity fix. That brought this down from 25 files to 10.Both #1122 and #1121 are left open for their authors. Planning unit:
devlog/_plan/260806_stacked_bug_campaign/050_phase6_anthropic_response_identity.md.Stack 6 of the 260806 attribution campaign, stacked on #1137.
Verification
bun test tests/response-model-identity.test.ts tests/images/loop.test.ts tests/web-search.test.ts tests/request-log.test.ts— 132 pass, 0 fail (re-run after rebase onto stack 5)bun run typecheck— exit 0bun run privacy:scan— passedChecklist