Skip to content

Commit 503b6e6

Browse files
committed
fix(bedrock): address PR feedback
1 parent c72dc16 commit 503b6e6

10 files changed

Lines changed: 257 additions & 44 deletions

File tree

packages/types/src/__tests__/bedrock.spec.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ import {
88
BEDROCK_NATIVE_1M_CONTEXT_MODEL_IDS,
99
bedrockModels,
1010
expandBedrockTargetsWith1MVariants,
11+
guessBedrockModelInfoFromId,
1112
hasBedrock1MContextIndicator,
13+
resolveBedrockMaxOutputTokensOverride,
1214
resolveBedrockModelInfo,
1315
stripBedrock1MContextSuffix,
1416
} from "../providers/bedrock.js"
@@ -65,6 +67,11 @@ describe("Bedrock model catalog", () => {
6567
expect((bedrockModels["anthropic.claude-haiku-4-5-20251001-v1:0"] as ModelInfo).promptCacheTtl).toBe("1h")
6668
expect((bedrockModels["anthropic.claude-opus-4-5-20251101-v1:0"] as ModelInfo).promptCacheTtl).toBe("1h")
6769
})
70+
71+
it("prefers specific guessed Opus patterns before generic Claude 4 patterns", () => {
72+
const guessed = guessBedrockModelInfoFromId("arn:aws:bedrock:us-west-2::foundation-model/claude-4-opus-custom")
73+
expect(guessed.maxTokens).toBe(4096)
74+
})
6875
})
6976

7077
describe("resolveBedrockModelInfo", () => {
@@ -94,6 +101,24 @@ describe("resolveBedrockModelInfo", () => {
94101
})
95102
expect(info.maxTokens).toBe(32_000)
96103
})
104+
105+
it("applies max-output overrides only to the target that was probed", () => {
106+
expect(
107+
resolveBedrockMaxOutputTokensOverride({
108+
currentTargetId: "global.anthropic.claude-opus-4-7",
109+
overrideTargetId: "global.anthropic.claude-opus-4-7",
110+
maxOutputTokensOverride: 128_000,
111+
}),
112+
).toBe(128_000)
113+
114+
expect(
115+
resolveBedrockMaxOutputTokensOverride({
116+
currentTargetId: "anthropic.claude-haiku-4-5-20251001-v1:0",
117+
overrideTargetId: "global.anthropic.claude-opus-4-7",
118+
maxOutputTokensOverride: 128_000,
119+
}),
120+
).toBeUndefined()
121+
})
97122
})
98123

99124
describe("expandBedrockTargetsWith1MVariants", () => {

packages/types/src/provider-settings.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,10 @@ const bedrockSchema = apiModelIdProviderModelSchema.extend({
236236
.optional(),
237237
awsModelContextWindow: z.number().optional(),
238238
// Empirically detected (or manually entered) per-config cap on the model's max output tokens.
239-
// Takes precedence over the static `bedrockModels.<id>.maxTokens` table when present.
239+
// Takes precedence over the static `bedrockModels.<id>.maxTokens` table when present
240+
// and scoped to the target id recorded in `awsModelMaxOutputTokensTargetId`.
240241
awsModelMaxOutputTokens: z.number().optional(),
242+
awsModelMaxOutputTokensTargetId: z.string().optional(),
241243
awsBedrockEndpointEnabled: z.boolean().optional(),
242244
awsBedrockEndpoint: z.string().optional(),
243245
awsBedrock1MContext: z.boolean().optional(), // Enable 'context-1m-2025-08-07' beta for 1M context window.

packages/types/src/providers/bedrock.ts

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -899,38 +899,38 @@ export const shouldUseBedrock1MContext = ({
899899

900900
export const guessBedrockModelInfoFromId = (modelId: string): Partial<ModelInfo> => {
901901
const modelConfigMap: Record<string, Partial<ModelInfo>> = {
902-
"claude-4": {
903-
maxTokens: 8192,
902+
"claude-4-opus": {
903+
maxTokens: 4096,
904904
contextWindow: 200_000,
905905
supportsImages: true,
906906
supportsPromptCache: true,
907907
},
908-
"claude-3-7": {
909-
maxTokens: 8192,
908+
"claude-3-opus": {
909+
maxTokens: 4096,
910910
contextWindow: 200_000,
911911
supportsImages: true,
912912
supportsPromptCache: true,
913913
},
914-
"claude-3-5": {
915-
maxTokens: 8192,
914+
"claude-3-haiku": {
915+
maxTokens: 4096,
916916
contextWindow: 200_000,
917917
supportsImages: true,
918918
supportsPromptCache: true,
919919
},
920-
"claude-4-opus": {
921-
maxTokens: 4096,
920+
"claude-4": {
921+
maxTokens: 8192,
922922
contextWindow: 200_000,
923923
supportsImages: true,
924924
supportsPromptCache: true,
925925
},
926-
"claude-3-opus": {
927-
maxTokens: 4096,
926+
"claude-3-7": {
927+
maxTokens: 8192,
928928
contextWindow: 200_000,
929929
supportsImages: true,
930930
supportsPromptCache: true,
931931
},
932-
"claude-3-haiku": {
933-
maxTokens: 4096,
932+
"claude-3-5": {
933+
maxTokens: 8192,
934934
contextWindow: 200_000,
935935
supportsImages: true,
936936
supportsPromptCache: true,
@@ -952,6 +952,24 @@ export const guessBedrockModelInfoFromId = (modelId: string): Partial<ModelInfo>
952952
}
953953
}
954954

955+
export const resolveBedrockMaxOutputTokensOverride = ({
956+
currentTargetId,
957+
overrideTargetId,
958+
maxOutputTokensOverride,
959+
}: {
960+
currentTargetId?: string
961+
overrideTargetId?: string
962+
maxOutputTokensOverride?: number
963+
}): number | undefined => {
964+
if (!currentTargetId || !overrideTargetId || !maxOutputTokensOverride || maxOutputTokensOverride <= 0) {
965+
return undefined
966+
}
967+
968+
return stripBedrock1MContextSuffix(currentTargetId) === stripBedrock1MContextSuffix(overrideTargetId)
969+
? maxOutputTokensOverride
970+
: undefined
971+
}
972+
955973
export const resolveBedrockModelInfo = ({
956974
baseModelId,
957975
targetId,
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"
2+
3+
const mocks = vi.hoisted(() => ({
4+
send: vi.fn(),
5+
sendOptions: [] as Array<{ abortSignal?: AbortSignal } | undefined>,
6+
}))
7+
8+
vi.mock("@aws-sdk/client-bedrock", () => {
9+
class BedrockClient {
10+
send(command: unknown, options?: { abortSignal?: AbortSignal }) {
11+
mocks.sendOptions.push(options)
12+
return mocks.send(command, options)
13+
}
14+
}
15+
16+
class ListFoundationModelsCommand {
17+
constructor(public readonly input: unknown) {}
18+
}
19+
20+
class ListInferenceProfilesCommand {
21+
constructor(public readonly input: unknown) {}
22+
}
23+
24+
return {
25+
BedrockClient,
26+
ListFoundationModelsCommand,
27+
ListInferenceProfilesCommand,
28+
}
29+
})
30+
31+
import { BEDROCK_DISCOVERY_TIMEOUT_MS, discoverBedrockTargets } from "../bedrock-discovery"
32+
33+
describe("discoverBedrockTargets", () => {
34+
beforeEach(() => {
35+
mocks.send.mockReset()
36+
mocks.sendOptions.length = 0
37+
})
38+
39+
afterEach(() => {
40+
vi.useRealTimers()
41+
})
42+
43+
it("times out and aborts slow AWS discovery calls", async () => {
44+
vi.useFakeTimers()
45+
mocks.send.mockImplementation(() => new Promise(() => {}))
46+
47+
const promise = expect(
48+
discoverBedrockTargets({
49+
awsRegion: "us-west-2",
50+
awsAccessKey: "AKIA",
51+
awsSecretKey: "secret",
52+
}),
53+
).rejects.toThrow(/Bedrock discovery timed out/)
54+
55+
await vi.advanceTimersByTimeAsync(BEDROCK_DISCOVERY_TIMEOUT_MS)
56+
57+
await promise
58+
expect(mocks.sendOptions).toHaveLength(2)
59+
expect(mocks.sendOptions.every((options) => options?.abortSignal?.aborted)).toBe(true)
60+
})
61+
})

src/api/providers/bedrock-discovery.ts

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import {
2121

2222
import { Package } from "../../shared/package"
2323

24+
export const BEDROCK_DISCOVERY_TIMEOUT_MS = 15_000
25+
2426
// The two AWS SDK packages we use here (`@aws-sdk/client-bedrock` for control-plane discovery
2527
// and `@aws-sdk/client-bedrock-runtime` for Converse probes) each ship their own discriminated
2628
// `Config` interface where `token`/`credentials` are tagged with package-private branded types.
@@ -152,7 +154,31 @@ const buildInferenceProfileTarget = (summary: InferenceProfileSummary): BedrockD
152154
}
153155
}
154156

155-
const listInferenceProfiles = async (client: BedrockClient) => {
157+
const withTimeout = async <T>(
158+
label: string,
159+
timeoutMs: number,
160+
operation: (abortSignal: AbortSignal) => Promise<T>,
161+
): Promise<T> => {
162+
const abortController = new AbortController()
163+
let timeoutId: ReturnType<typeof setTimeout> | undefined
164+
165+
const timeoutPromise = new Promise<never>((_, reject) => {
166+
timeoutId = setTimeout(() => {
167+
abortController.abort()
168+
reject(new Error(`${label} timed out after ${timeoutMs}ms`))
169+
}, timeoutMs)
170+
})
171+
172+
try {
173+
return await Promise.race([operation(abortController.signal), timeoutPromise])
174+
} finally {
175+
if (timeoutId) {
176+
clearTimeout(timeoutId)
177+
}
178+
}
179+
}
180+
181+
const listInferenceProfiles = async (client: BedrockClient, abortSignal?: AbortSignal) => {
156182
const results: InferenceProfileSummary[] = []
157183
let nextToken: string | undefined
158184

@@ -162,6 +188,7 @@ const listInferenceProfiles = async (client: BedrockClient) => {
162188
nextToken,
163189
maxResults: 100,
164190
}),
191+
{ abortSignal },
165192
)
166193

167194
results.push(...(response.inferenceProfileSummaries ?? []))
@@ -178,10 +205,15 @@ export const discoverBedrockTargets = async (options: ProviderSettings): Promise
178205

179206
const client = new BedrockClient(toBedrockClientConfig(options))
180207

181-
const [foundationModelsResponse, inferenceProfiles] = await Promise.all([
182-
client.send(new ListFoundationModelsCommand({})),
183-
listInferenceProfiles(client),
184-
])
208+
const [foundationModelsResponse, inferenceProfiles] = await withTimeout(
209+
"Bedrock discovery",
210+
BEDROCK_DISCOVERY_TIMEOUT_MS,
211+
(abortSignal) =>
212+
Promise.all([
213+
client.send(new ListFoundationModelsCommand({}), { abortSignal }),
214+
listInferenceProfiles(client, abortSignal),
215+
]),
216+
)
185217

186218
const targets = [
187219
...(foundationModelsResponse.modelSummaries ?? [])

src/api/providers/bedrock.ts

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ import {
3434
ApiProviderError,
3535
inferBedrockInvokeTargetKind,
3636
parseBedrockBaseModelId,
37+
resolveBedrockInvokeTargetId,
38+
resolveBedrockMaxOutputTokensOverride,
3739
resolveBedrockModelInfo,
3840
shouldUseBedrock1MContext,
3941
stripBedrock1MContextSuffix,
@@ -379,7 +381,9 @@ export class AwsBedrockHandler extends BaseProvider implements SingleCompletionH
379381
// parseBaseModelId strips cross-region inference prefixes (e.g. `us.`, `eu.`) and the
380382
// synthetic `:1m` dropdown suffix.
381383
const baseModelId = this.parseBaseModelId(modelConfig.id)
382-
const requiresAdaptiveThinking = BEDROCK_ADAPTIVE_THINKING_MODEL_IDS.includes(baseModelId as any)
384+
const requiresAdaptiveThinking = BEDROCK_ADAPTIVE_THINKING_MODEL_IDS.includes(
385+
baseModelId as (typeof BEDROCK_ADAPTIVE_THINKING_MODEL_IDS)[number],
386+
)
383387

384388
// Determine if thinking should be enabled
385389
// metadata?.thinking?.enabled: Explicitly enabled through API metadata (direct request)
@@ -443,7 +447,7 @@ export class AwsBedrockHandler extends BaseProvider implements SingleCompletionH
443447
const configuredTargetForIndicator =
444448
this.options.awsBedrockInvokeTarget || this.options.awsCustomArn || modelConfig.id
445449
const is1MContextEnabled =
446-
BEDROCK_1M_CONTEXT_MODEL_IDS.includes(baseModelId as any) &&
450+
BEDROCK_1M_CONTEXT_MODEL_IDS.includes(baseModelId as (typeof BEDROCK_1M_CONTEXT_MODEL_IDS)[number]) &&
447451
shouldUseBedrock1MContext({
448452
targetId: configuredTargetForIndicator,
449453
baseModelId,
@@ -456,11 +460,14 @@ export class AwsBedrockHandler extends BaseProvider implements SingleCompletionH
456460
// the fine-grained-tool-streaming beta, so we must omit anthropic_beta entirely
457461
// for those models. Older Claudes silently accept (and effectively ignore) them,
458462
// so we keep the current behavior for them.
459-
const skipAnthropicBetaFlags = BEDROCK_NATIVE_1M_CONTEXT_MODEL_IDS.includes(baseModelId as any)
463+
const skipAnthropicBetaFlags = BEDROCK_NATIVE_1M_CONTEXT_MODEL_IDS.includes(
464+
baseModelId as (typeof BEDROCK_NATIVE_1M_CONTEXT_MODEL_IDS)[number],
465+
)
460466

461467
// Determine if service tier should be applied (checked later when building payload)
462468
const useServiceTier =
463-
this.options.awsBedrockServiceTier && BEDROCK_SERVICE_TIER_MODEL_IDS.includes(baseModelId as any)
469+
this.options.awsBedrockServiceTier &&
470+
BEDROCK_SERVICE_TIER_MODEL_IDS.includes(baseModelId as (typeof BEDROCK_SERVICE_TIER_MODEL_IDS)[number])
464471
if (useServiceTier) {
465472
logger.info("Service tier specified for Bedrock request", {
466473
ctx: "bedrock",
@@ -1133,6 +1140,14 @@ export class AwsBedrockHandler extends BaseProvider implements SingleCompletionH
11331140
return parseBedrockBaseModelId(modelId)
11341141
}
11351142

1143+
private getScopedMaxOutputTokensOverride(): number | undefined {
1144+
return resolveBedrockMaxOutputTokensOverride({
1145+
currentTargetId: resolveBedrockInvokeTargetId(this.options),
1146+
overrideTargetId: this.options.awsModelMaxOutputTokensTargetId,
1147+
maxOutputTokensOverride: this.options.awsModelMaxOutputTokens,
1148+
})
1149+
}
1150+
11361151
//Prompt Router responses come back in a different sequence and the model used is in the response and must be fetched by name
11371152
getModelById(modelId: string, modelType?: string): { id: BedrockModelId | string; info: ModelInfo } {
11381153
let model
@@ -1143,8 +1158,8 @@ export class AwsBedrockHandler extends BaseProvider implements SingleCompletionH
11431158
modelMaxTokens: this.options.modelMaxTokens,
11441159
contextWindowOverride: this.options.awsModelContextWindow,
11451160
// Empirically detected per-config max output tokens (from the "Detect" probe in the
1146-
// settings UI) widens the static cap so downstream request builders pick it up too.
1147-
maxOutputTokensOverride: this.options.awsModelMaxOutputTokens,
1161+
// settings UI) widens the static cap only for the exact AWS target that was probed.
1162+
maxOutputTokensOverride: this.getScopedMaxOutputTokensOverride(),
11481163
})
11491164

11501165
if (resolved.baseModelId in bedrockModels) {
@@ -1227,7 +1242,9 @@ export class AwsBedrockHandler extends BaseProvider implements SingleCompletionH
12271242
const baseIdForGlobal = this.parseBaseModelId(modelConfig.id)
12281243
if (
12291244
this.options.awsUseGlobalInference &&
1230-
BEDROCK_GLOBAL_INFERENCE_MODEL_IDS.includes(baseIdForGlobal as any)
1245+
BEDROCK_GLOBAL_INFERENCE_MODEL_IDS.includes(
1246+
baseIdForGlobal as (typeof BEDROCK_GLOBAL_INFERENCE_MODEL_IDS)[number],
1247+
)
12311248
) {
12321249
modelConfig.id = `global.${baseIdForGlobal}`
12331250
}
@@ -1244,7 +1261,10 @@ export class AwsBedrockHandler extends BaseProvider implements SingleCompletionH
12441261
// Check if 1M context is enabled for supported Claude 4 models
12451262
// Use parseBaseModelId to handle cross-region inference prefixes
12461263
const baseModelId = this.parseBaseModelId(modelConfig.id)
1247-
if (BEDROCK_1M_CONTEXT_MODEL_IDS.includes(baseModelId as any) && this.options.awsBedrock1MContext) {
1264+
if (
1265+
BEDROCK_1M_CONTEXT_MODEL_IDS.includes(baseModelId as (typeof BEDROCK_1M_CONTEXT_MODEL_IDS)[number]) &&
1266+
this.options.awsBedrock1MContext
1267+
) {
12481268
// Update context window and pricing to 1M tier when 1M context beta is enabled
12491269
const tier = modelConfig.info.tiers?.[0]
12501270
modelConfig.info = {
@@ -1268,7 +1288,12 @@ export class AwsBedrockHandler extends BaseProvider implements SingleCompletionH
12681288

12691289
// Apply service tier pricing if specified and model supports it
12701290
const baseModelIdForTier = this.parseBaseModelId(modelConfig.id)
1271-
if (this.options.awsBedrockServiceTier && BEDROCK_SERVICE_TIER_MODEL_IDS.includes(baseModelIdForTier as any)) {
1291+
if (
1292+
this.options.awsBedrockServiceTier &&
1293+
BEDROCK_SERVICE_TIER_MODEL_IDS.includes(
1294+
baseModelIdForTier as (typeof BEDROCK_SERVICE_TIER_MODEL_IDS)[number],
1295+
)
1296+
) {
12721297
const pricingMultiplier = BEDROCK_SERVICE_TIER_PRICING[this.options.awsBedrockServiceTier]
12731298
if (pricingMultiplier && pricingMultiplier !== 1.0) {
12741299
// Apply pricing multiplier to all price fields

0 commit comments

Comments
 (0)