Add Claude Sonnet 5 default#134
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
github-actions
Bot
added
vouch:trusted
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47de4ca871
ℹ️ 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".
| id: "contextWindow", | ||
| label: "Context Window", | ||
| options: [ | ||
| { value: "200k", label: "200k", isDefault: true }, |
There was a problem hiding this comment.
For Claude Code versions that support Sonnet 5, the changelog describes it as having a native 1M-token context window (https://code.claude.com/docs/en/changelog), but this descriptor defaults the model to a synthetic 200k/1m selector. In the default path resolveClaudeApiModelId sends the plain slug while selectedClaudeContextWindow reports 200k from this option, so token/context usage and the traits UI understate the actual window; selecting 1M also emits claude-sonnet-5[1m] instead of using the native model, unlike the existing Opus 4.7/4.8 native-1M handling.
Useful? React with 👍 / 👎.
|
@coderabbitai review |
✅ Action performedReview finished.
|
730be69 to
4798ab9
Compare
47de4ca to
fcef224
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fcef224c3b
ℹ️ 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".
| Effect.provide( | ||
| mockSpawnerLayer((args) => { | ||
| const joined = args.join(" "); | ||
| if (joined === "--version") return { stdout: "1.0.0\n", stderr: "", code: 0 }; |
There was a problem hiding this comment.
In this test, the mocked claude --version returns 1.0.0, but the provider code added in this commit filters claude-sonnet-5 unless the parsed version is at least 2.1.197. When ProviderRegistry.test.ts runs, checkClaudeProviderStatus therefore returns no Sonnet 5 entry and the assertion above this stub fails before any descriptor checks run; bump the stubbed version or change the expectation to match the gating behavior.
Useful? React with 👍 / 👎.
| getClaudeModelCapabilities("claude-sonnet-5").optionDescriptors?.some( | ||
| (descriptor) => descriptor.id === "contextWindow", | ||
| ), | ||
| true, |
There was a problem hiding this comment.
After using a supported Claude Code version, this assertion still fails because the Sonnet 5 model definition in this commit only exposes the effort descriptor and intentionally has no contextWindow selector. Update this test to assert the native/no-selector behavior, or use a different model if the test is meant to cover selectable context windows.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0acab3dda
ℹ️ 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".
a0acab3 to
d7b1bb2
Compare
4798ab9 to
6258f37
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7b1bb2a77
ℹ️ 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".
| "claude-opus-4.6": "claude-opus-4-6", | ||
| "claude-opus-4-6-20251117": "claude-opus-4-6", | ||
| sonnet: "claude-sonnet-4-6", | ||
| sonnet: "claude-sonnet-5", |
There was a problem hiding this comment.
When Claude Code is older than v2.1.197 and a user already has sonnet in customModels, providerModelsFromSettings normalizes that custom entry through this new alias after getBuiltInClaudeModelsForVersion has removed the built-in claude-sonnet-5; because the filtered built-in list no longer has that slug in seen, it gets appended back as a custom model and the new default/fallback path can select a model the installed CLI cannot run. Fresh evidence beyond the prior built-in gating finding is this custom-model normalization path reintroducing the gated slug.
Useful? React with 👍 / 👎.
| case "claude-sonnet-5": | ||
| return 1_000_000; |
There was a problem hiding this comment.
This unconditional 1M seed is wrong when Claude Code is running with CLAUDE_CODE_DISABLE_1M_CONTEXT=1 or behind an ANTHROPIC_BASE_URL gateway: the Claude Code docs for Sonnet 5 context say those configurations budget the window at 200K unless the 1M variant is selected (https://code.claude.com/docs/en/model-config#sonnet-5-context-window). In those environments T3 will report maxTokens: 1_000_000 and clamp usage against 1M even though the session auto-compacts around 200K, so derive the seeded window from the Claude environment/model variant instead of hard-coding the slug.
Useful? React with 👍 / 👎.
6258f37 to
eeacb89
Compare
d7b1bb2 to
61dfc95
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61dfc954b9
ℹ️ 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".
| "claude-opus-4.6": "claude-opus-4-6", | ||
| "claude-opus-4-6-20251117": "claude-opus-4-6", | ||
| sonnet: "claude-sonnet-4-6", | ||
| sonnet: "claude-sonnet-5", |
There was a problem hiding this comment.
When a Claude instance is configured with a non-Anthropic backend via per-instance environment variables such as ANTHROPIC_BASE_URL, Claude Code resolves sonnet differently by backend; the official model-config docs say Anthropic API maps it to Sonnet 5, while AWS maps it to Sonnet 4.6 and Bedrock/Vertex/Foundry map it to Sonnet 4.5 (https://code.claude.com/docs/en/model-config). This global normalization rewrites any saved/custom sonnet to claude-sonnet-5 before the CLI can apply that backend-specific alias, so those instances can dispatch an unavailable full model instead of their supported Sonnet default.
Useful? React with 👍 / 👎.
61dfc95 to
29b3f3c
Compare
…20260701-claude-sonnet-5
…20260701-claude-sonnet-5
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f09beb8605
ℹ️ 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".
| { value: "200k", label: "200k", isDefault: true }, | ||
| { value: "1m", label: "1M" }, |
There was a problem hiding this comment.
Fresh evidence in this revision is that the Sonnet 5 descriptor now marks 200k as the default context selection; the composer materializes descriptor defaults into modelSelection, so normal direct-Anthropic claude-sonnet-5 sends carry contextWindow=200k and selectedClaudeContextWindow returns 200,000 before reaching its Sonnet 5 native-1M branch. Claude Code's model config docs say Anthropic API Sonnet 5 has no 200K variant and always runs with the 1M window (https://code.claude.com/docs/en/model-config#sonnet-5-context-window), so the default path under-reports/clamps usage and exposes a non-existent 200K choice.
Useful? React with 👍 / 👎.
…stream-20260701-chat-mobile-review-fix
…c/upstream-20260701-claude-sonnet-5-review-fix
8e8ba2e to
9ee152a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ced218f852
ℹ️ 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".
|
|
||
| export const makePendingClaudeProvider = ( | ||
| claudeSettings: ClaudeSettings, | ||
| environment: NodeJS.ProcessEnv = process.env, |
There was a problem hiding this comment.
With a custom Claude instance whose provider-instance environment sets ANTHROPIC_BASE_URL or CLAUDE_CODE_DISABLE_1M_CONTEXT, this default reads only the server's global env: I checked ClaudeDriver.create, where the initial snapshot calls makePendingClaudeProvider(settings.provider) while checked snapshots pass the merged processEnv. That makes the pending model list omit Sonnet 5's contextWindow selector until the probe finishes, so a user who opens or sends immediately on a gateway instance cannot choose/report the 1M sonnet[1m] variant even though the later snapshot exposes it; pass the merged instance env into the pending snapshot.
Useful? React with 👍 / 👎.
| return ( | ||
| environment.CLAUDE_CODE_DISABLE_1M_CONTEXT === "1" || | ||
| (environment.ANTHROPIC_BASE_URL?.trim().length ?? 0) > 0 |
There was a problem hiding this comment.
When a Claude instance sets CLAUDE_CODE_DISABLE_1M_CONTEXT=1, this treats it the same as an LLM gateway, and withClaudeSonnet5ContextWindowSelector then appends the shared selector that includes 1m; selecting that option later dispatches sonnet[1m]. I checked the Claude Code model-config docs, which say this env var removes 1M variants from the picker (https://code.claude.com/docs/en/model-config#extended-context), so T3 can bypass an operator's context cap or send a variant they explicitly disabled; only expose the 1M choice for ANTHROPIC_BASE_URL, not for the disable flag.
Useful? React with 👍 / 👎.
53ee88c to
feb7380
Compare
Merge activity
|
1 participant
What Changed
Adds Claude Sonnet 5 support and default behavior translated into the fork's provider-instance model system.
Why
This ports the upstream Claude Sonnet 5 default while preserving the fork's custom provider and model configuration architecture.
UI Changes
None beyond provider/model option behavior already covered by the model default update.
Checklist
vp checkvp run typecheckvp run lint:mobilevp testhad one ACP timeout flake; rerunning that file passed