fix(anthropic): honor custom apiModelId instead of silently defaulting to claude-sonnet-4-5#842
Conversation
…g to claude-sonnet-4-5 Unrecognized model IDs were coerced to the hardcoded default before being sent to the API and for capability lookups, breaking custom deployments and picking the wrong thinking config. Fixes Zoo-Code-Org#418
grizmin
requested review from
JamesRobert20,
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe Anthropic provider now preserves unrecognized ChangesCustom apiModelId fallback fix
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant AnthropicHandler
participant guessModelInfoFromId
participant AnthropicSDK
AnthropicHandler->>AnthropicHandler: resolve apiModelId
alt custom id is known
AnthropicHandler->>AnthropicHandler: use anthropicModels[id]
else custom id is unknown
AnthropicHandler->>guessModelInfoFromId: infer ModelInfo from modelId
guessModelInfoFromId-->>AnthropicHandler: guessed info or default info
end
AnthropicHandler->>AnthropicSDK: send request with preserved model id and thinking shape
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
github-actions
Bot
added
the
awaiting-review
navedmerchant
left a comment
navedmerchant
left a comment
There was a problem hiding this comment.
Thanks for this fix! Couple of small comments and should be good to merge
| // Guesses capabilities for an unrecognized model ID via known-family substring match. | ||
| private guessModelInfoFromId(modelId: string): ModelInfo { | ||
| const matchedId = (Object.keys(anthropicModels) as AnthropicModelId[]) | ||
| .sort((a, b) => b.length - a.length) |
There was a problem hiding this comment.
lets hoist a sorted list out of this method so we dont have to needlessly sort every time
| private guessModelInfoFromId(modelId: string): ModelInfo { | ||
| const matchedId = (Object.keys(anthropicModels) as AnthropicModelId[]) | ||
| .sort((a, b) => b.length - a.length) | ||
| .find((knownId) => modelId.includes(knownId)) |
There was a problem hiding this comment.
we should lowercase the modelId before matching so the matching is case insensitve
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/api/providers/anthropic.ts (1)
356-363: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSubstring matching is case-sensitive — custom IDs with non-lowercase casing won't match known families.
modelId.includes(knownId)is case-sensitive, so a custom deployment ID likeClaude-Sonnet-5-BFwould fail to matchclaude-sonnet-5and fall back to the default model's capabilities instead of the correct Sonnet 5 family. This was flagged in a previous review and remains unaddressed.🛡️ Proposed fix for case-insensitive matching
private guessModelInfoFromId(modelId: string): ModelInfo { - const matchedId = (Object.keys(anthropicModels) as AnthropicModelId[]) - .sort((a, b) => b.length - a.length) - .find((knownId) => modelId.includes(knownId)) + const lowerModelId = modelId.toLowerCase() + const matchedId = (Object.keys(anthropicModels) as AnthropicModelId[]) + .sort((a, b) => b.length - a.length) + .find((knownId) => lowerModelId.includes(knownId))🤖 Prompt for 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. In `@src/api/providers/anthropic.ts` around lines 356 - 363, The capability lookup in guessModelInfoFromId is still using case-sensitive substring matching, so custom model IDs with different casing can miss the right family and fall back to the default. Update the matching logic in anthopicModels lookup to compare normalized casing for both modelId and each knownId before calling includes, while keeping the longest-match ordering and returning the same ModelInfo fallback behavior.
🧹 Nitpick comments (1)
src/api/providers/anthropic.ts (1)
365-374: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftCustom model IDs now bypass prompt caching and 1M context beta in
createMessage.
getModel()correctly preserves the customapiModelId, butcreateMessage(lines 91–228) and the 1M context check (lines 76–84) match on the rawmodelIdvia switch/case against known IDs only. A custom ID likeclaude-sonnet-5-bffalls to thedefaultcase, which omitscache_controlbreakpoints and theprompt-caching-2024-07-31beta header. This isn't a regression (the old behavior sent the wrong model and failed), but it's a missed optimization for custom deployments of caching-capable models.Consider deriving caching decisions from the guessed model family rather than the raw custom ID, either now or as a follow-up.
🤖 Prompt for 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. In `@src/api/providers/anthropic.ts` around lines 365 - 374, `AnthropicProvider.createMessage` and the 1M-context gating currently branch on the raw `modelId`, so custom `apiModelId` values skip prompt caching and the `prompt-caching-2024-07-31` beta even when they map to a caching-capable Claude family. Update the model-selection logic to derive these feature decisions from the resolved/guessed model info returned by `getModel()` or `guessModelInfoFromId`, and use that in the switch/case and 1M-context check instead of only matching known IDs. Keep custom IDs preserved, but ensure `createMessage` still enables cache breakpoints and beta headers when the guessed family supports them.
🤖 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.
Duplicate comments:
In `@src/api/providers/anthropic.ts`:
- Around line 356-363: The capability lookup in guessModelInfoFromId is still
using case-sensitive substring matching, so custom model IDs with different
casing can miss the right family and fall back to the default. Update the
matching logic in anthopicModels lookup to compare normalized casing for both
modelId and each knownId before calling includes, while keeping the
longest-match ordering and returning the same ModelInfo fallback behavior.
---
Nitpick comments:
In `@src/api/providers/anthropic.ts`:
- Around line 365-374: `AnthropicProvider.createMessage` and the 1M-context
gating currently branch on the raw `modelId`, so custom `apiModelId` values skip
prompt caching and the `prompt-caching-2024-07-31` beta even when they map to a
caching-capable Claude family. Update the model-selection logic to derive these
feature decisions from the resolved/guessed model info returned by `getModel()`
or `guessModelInfoFromId`, and use that in the switch/case and 1M-context check
instead of only matching known IDs. Keep custom IDs preserved, but ensure
`createMessage` still enables cache breakpoints and beta headers when the
guessed family supports them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 77b292ba-936a-4be7-a501-4f8d4bcec7ae
📒 Files selected for processing (3)
.changeset/fix-anthropic-custom-model-id-fallback.mdsrc/api/providers/__tests__/anthropic.spec.tssrc/api/providers/anthropic.ts
… ID matching case insensitive in Anthropic
2 participants
Summary
AnthropicHandler.getModel()coerced anyapiModelIdnot present in the staticanthropicModelstable down toanthropicDefaultModelId("claude-sonnet-4-5") — and that coerced id is what actually got sent asmodelin the API request, silently ignoring a user-configured custom model name.thinkingrequest param: an unrecognized id inherited the default model's (older) capability profile, so custom deployments of newer models (e.g. Sonnet 5 class) got the legacythinking: {type: "enabled", budget_tokens}shape instead of{type: "adaptive"}, which those models reject with a 400.apiModelId. For unrecognized values, capabilities are best-effort guessed via known model-family substring matching (same pattern as the existingBedrockHandler.guessModelInfoFromId).Fixes #418
Fixes #843
Test plan
anthropic.spec.tscovering: custom model id preserved ingetModel().id, capability guessing via substring match, fallback to default info when no family matches, andcreateMessagesending the raw custom id withthinking: {type: "adaptive"}.pnpm --filter zoo-code test- 50/50 passingpnpm --filter zoo-code check-types- cleanpnpm --filter zoo-code lint- cleanSummary by CodeRabbit