Conversation
daniel-lxs
approved these changes
Sep 30, 2025
| 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], |
Contributor
There was a problem hiding this comment.
[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.
| const apiConfiguration: ProviderSettings = { | ||
| apiProvider: "bedrock", | ||
| apiModelId: BEDROCK_CLAUDE_SONNET_4_MODEL_ID, | ||
| apiModelId: BEDROCK_1M_CONTEXT_MODEL_IDS[0], |
Contributor
There was a problem hiding this comment.
[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.
| if ( | ||
| provider === "anthropic" && | ||
| id === "claude-sonnet-4-20250514" && | ||
| (id === "claude-sonnet-4-20250514" || id === "claude-sonnet-4-5") && |
Contributor
There was a problem hiding this comment.
[P2] Robustness: selecting tiers?.[0] assumes the 1M tier is always first. Prefer selecting the highest-contextWindow tier to avoid order-dependence.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
Refactor Claude Sonnet 4 model handling to use
BEDROCK_1M_CONTEXT_MODEL_IDSfor 1M context window support.BEDROCK_CLAUDE_SONNET_4_MODEL_IDconstant frombedrock.ts.AwsBedrockHandlerinbedrock.tsto useBEDROCK_1M_CONTEXT_MODEL_IDS[0]for 1M context window logic.getSelectedModel()inuseSelectedModel.tsto handle 1M context for Claude Sonnet 4 usingBEDROCK_1M_CONTEXT_MODEL_IDS.bedrock.spec.tsto useBEDROCK_1M_CONTEXT_MODEL_IDS[0]instead ofBEDROCK_CLAUDE_SONNET_4_MODEL_ID.useSelectedModel.spec.tsto reflect changes in model ID handling for 1M context.This description was created by
for e056367. You can customize this summary. It will automatically update as commits are pushed.