Skip to content

A couple more sonnet 4.5 fixes #8421

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 30, 2025
Merged

A couple more sonnet 4.5 fixes #8421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/types/src/providers/bedrock.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,6 @@ export const BEDROCK_REGIONS = [
{ value: "us-gov-west-1", label: "us-gov-west-1" },
].sort((a, b) => a.value.localeCompare(b.value))

export const BEDROCK_CLAUDE_SONNET_4_MODEL_ID = "anthropic.claude-sonnet-4-20250514-v1:0"
export const BEDROCK_1M_CONTEXT_MODEL_IDS = [
"anthropic.claude-sonnet-4-20250514-v1:0",
"anthropic.claude-sonnet-4-5-20250929-v1:0",
Expand Down
18 changes: 9 additions & 9 deletions src/api/providers/__tests__/bedrock.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ vi.mock("@aws-sdk/client-bedrock-runtime", () => {

import { AwsBedrockHandler } from "../bedrock"
import { ConverseStreamCommand, BedrockRuntimeClient } from "@aws-sdk/client-bedrock-runtime"
import { BEDROCK_CLAUDE_SONNET_4_MODEL_ID } from "@roo-code/types"
import { BEDROCK_1M_CONTEXT_MODEL_IDS } from "@roo-code/types"

import type { Anthropic } from "@anthropic-ai/sdk"

Expand Down Expand Up @@ -569,7 +569,7 @@ describe("AwsBedrockHandler", () => {
describe("1M context beta feature", () => {
it("should enable 1M context window when awsBedrock1MContext is true for Claude Sonnet 4", () => {
const handler = new AwsBedrockHandler({
apiModelId: BEDROCK_CLAUDE_SONNET_4_MODEL_ID,
apiModelId: BEDROCK_1M_CONTEXT_MODEL_IDS[0],
Copy link

@roomote roomote bot Sep 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Tests rely on BEDROCK_1M_CONTEXT_MODEL_IDS[0]. Suggest adding a test case for the 4.5 entry (index 1) or refactoring tests to avoid index assumptions so we won't regress if the array order changes.

awsAccessKey: "test",
awsSecretKey: "test",
awsRegion: "us-east-1",
Expand All @@ -584,7 +584,7 @@ describe("AwsBedrockHandler", () => {

it("should use default context window when awsBedrock1MContext is false for Claude Sonnet 4", () => {
const handler = new AwsBedrockHandler({
apiModelId: BEDROCK_CLAUDE_SONNET_4_MODEL_ID,
apiModelId: BEDROCK_1M_CONTEXT_MODEL_IDS[0],
awsAccessKey: "test",
awsSecretKey: "test",
awsRegion: "us-east-1",
Expand Down Expand Up @@ -614,7 +614,7 @@ describe("AwsBedrockHandler", () => {

it("should include anthropic_beta parameter when 1M context is enabled", async () => {
const handler = new AwsBedrockHandler({
apiModelId: BEDROCK_CLAUDE_SONNET_4_MODEL_ID,
apiModelId: BEDROCK_1M_CONTEXT_MODEL_IDS[0],
awsAccessKey: "test",
awsSecretKey: "test",
awsRegion: "us-east-1",
Expand Down Expand Up @@ -644,7 +644,7 @@ describe("AwsBedrockHandler", () => {

it("should not include anthropic_beta parameter when 1M context is disabled", async () => {
const handler = new AwsBedrockHandler({
apiModelId: BEDROCK_CLAUDE_SONNET_4_MODEL_ID,
apiModelId: BEDROCK_1M_CONTEXT_MODEL_IDS[0],
awsAccessKey: "test",
awsSecretKey: "test",
awsRegion: "us-east-1",
Expand Down Expand Up @@ -698,7 +698,7 @@ describe("AwsBedrockHandler", () => {

it("should enable 1M context window with cross-region inference for Claude Sonnet 4", () => {
const handler = new AwsBedrockHandler({
apiModelId: BEDROCK_CLAUDE_SONNET_4_MODEL_ID,
apiModelId: BEDROCK_1M_CONTEXT_MODEL_IDS[0],
awsAccessKey: "test",
awsSecretKey: "test",
awsRegion: "us-east-1",
Expand All @@ -711,12 +711,12 @@ describe("AwsBedrockHandler", () => {
// Should have 1M context window even with cross-region prefix
expect(model.info.contextWindow).toBe(1_000_000)
// Model ID should have cross-region prefix
expect(model.id).toBe(`us.${BEDROCK_CLAUDE_SONNET_4_MODEL_ID}`)
expect(model.id).toBe(`us.${BEDROCK_1M_CONTEXT_MODEL_IDS[0]}`)
})

it("should include anthropic_beta parameter with cross-region inference for Claude Sonnet 4", async () => {
const handler = new AwsBedrockHandler({
apiModelId: BEDROCK_CLAUDE_SONNET_4_MODEL_ID,
apiModelId: BEDROCK_1M_CONTEXT_MODEL_IDS[0],
awsAccessKey: "test",
awsSecretKey: "test",
awsRegion: "us-east-1",
Expand Down Expand Up @@ -746,7 +746,7 @@ describe("AwsBedrockHandler", () => {
// Should not include anthropic_version since thinking is not enabled
expect(commandArg.additionalModelRequestFields.anthropic_version).toBeUndefined()
// Model ID should have cross-region prefix
expect(commandArg.modelId).toBe(`us.${BEDROCK_CLAUDE_SONNET_4_MODEL_ID}`)
expect(commandArg.modelId).toBe(`us.${BEDROCK_1M_CONTEXT_MODEL_IDS[0]}`)
})
})
})
3 changes: 1 addition & 2 deletions src/api/providers/bedrock.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
BEDROCK_MAX_TOKENS,
BEDROCK_DEFAULT_CONTEXT,
AWS_INFERENCE_PROFILE_MAPPING,
BEDROCK_CLAUDE_SONNET_4_MODEL_ID,
BEDROCK_1M_CONTEXT_MODEL_IDS,
} from "@roo-code/types"

Expand Down Expand Up @@ -373,7 +372,7 @@ export class AwsBedrockHandler extends BaseProvider implements SingleCompletionH
maxTokens: modelConfig.maxTokens || (modelConfig.info.maxTokens as number),
temperature: modelConfig.temperature ?? (this.options.modelTemperature as number),
}

// Check if 1M context is enabled for Claude Sonnet 4
// Use parseBaseModelId to handle cross-region inference prefixes
const baseModelId = this.parseBaseModelId(modelConfig.id)
Expand Down
10 changes: 5 additions & 5 deletions webview-ui/src/components/ui/hooks/__tests__/useSelectedModel.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"
import { renderHook } from "@testing-library/react"
import type { Mock } from "vitest"

import { ProviderSettings, ModelInfo, BEDROCK_CLAUDE_SONNET_4_MODEL_ID } from "@roo-code/types"
import { ProviderSettings, ModelInfo, BEDROCK_1M_CONTEXT_MODEL_IDS } from "@roo-code/types"

import { useSelectedModel } from "../useSelectedModel"
import { useRouterModels } from "../useRouterModels"
Expand Down Expand Up @@ -474,28 +474,28 @@ describe("useSelectedModel", () => {
it("should enable 1M context window for Bedrock Claude Sonnet 4 when awsBedrock1MContext is true", () => {
const apiConfiguration: ProviderSettings = {
apiProvider: "bedrock",
apiModelId: BEDROCK_CLAUDE_SONNET_4_MODEL_ID,
apiModelId: BEDROCK_1M_CONTEXT_MODEL_IDS[0],
Copy link

@roomote roomote bot Sep 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Similar to the Bedrock tests, this relies on BEDROCK_1M_CONTEXT_MODEL_IDS[0]. Consider adding a corresponding test for the 4.5 model or removing reliance on array order in tests.

awsBedrock1MContext: true,
}

const wrapper = createWrapper()
const { result } = renderHook(() => useSelectedModel(apiConfiguration), { wrapper })

expect(result.current.id).toBe(BEDROCK_CLAUDE_SONNET_4_MODEL_ID)
expect(result.current.id).toBe(BEDROCK_1M_CONTEXT_MODEL_IDS[0])
expect(result.current.info?.contextWindow).toBe(1_000_000)
})

it("should use default context window for Bedrock Claude Sonnet 4 when awsBedrock1MContext is false", () => {
const apiConfiguration: ProviderSettings = {
apiProvider: "bedrock",
apiModelId: BEDROCK_CLAUDE_SONNET_4_MODEL_ID,
apiModelId: BEDROCK_1M_CONTEXT_MODEL_IDS[0],
awsBedrock1MContext: false,
}

const wrapper = createWrapper()
const { result } = renderHook(() => useSelectedModel(apiConfiguration), { wrapper })

expect(result.current.id).toBe(BEDROCK_CLAUDE_SONNET_4_MODEL_ID)
expect(result.current.id).toBe(BEDROCK_1M_CONTEXT_MODEL_IDS[0])
expect(result.current.info?.contextWindow).toBe(200_000)
})

Expand Down
4 changes: 2 additions & 2 deletions webview-ui/src/components/ui/hooks/useSelectedModel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,11 +367,11 @@ function getSelectedModel({
// Apply 1M context beta tier pricing for Claude Sonnet 4
if (
provider === "anthropic" &&
id === "claude-sonnet-4-20250514" &&
(id === "claude-sonnet-4-20250514" || id === "claude-sonnet-4-5") &&
Copy link

@roomote roomote bot Sep 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Robustness: selecting tiers?.[0] assumes the 1M tier is always first. Prefer selecting the highest-contextWindow tier to avoid order-dependence.

apiConfiguration.anthropicBeta1MContext &&
baseInfo
) {
// Type assertion since we know claude-sonnet-4-20250514 has tiers
// Type assertion since we know claude-sonnet-4-20250514 and claude-sonnet-4-5 have tiers
const modelWithTiers = baseInfo as typeof baseInfo & {
tiers?: Array<{
contextWindow: number
Expand Down
Loading