fix: remove topP parameter for Bedrock models to fix Sonnet 4.5 compatibility #8378
Conversation
…tibility - AWS Bedrock doesn't allow both temperature and topP parameters to be specified simultaneously - Removed topP parameter from inference config for all Bedrock models - This fixes the 'temperature and top_p cannot both be specified' error for Claude Sonnet 3.5 v2 (4.5) - Added comprehensive tests to verify the fix Fixes #8377
dosubot
bot
added
size:L
hannesrudolph
added
the
Issue/PR - Triage
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| if (!thinkingEnabled) { | ||
| inferenceConfig.topP = 0.1 | ||
| } | ||
| // AWS Bedrock doesn't allow both temperature and topP to be specified for certain models |
There was a problem hiding this comment.
P2: Nice fix. Consider clarifying in a short comment whether "skip topP" is intended as a permanent rule across all Bedrock models, not just Anthropic/Claude. This helps future maintainers avoid reintroducing it in other call paths.
| maxTokens: modelConfig.maxTokens || (modelConfig.info.maxTokens as number), | ||
| temperature: modelConfig.temperature ?? (this.options.modelTemperature as number), | ||
| ...(thinkingEnabled ? {} : { topP: 0.1 }), // Only set topP when thinking is NOT enabled | ||
| // AWS Bedrock doesn't allow both temperature and topP to be specified for certain models |
There was a problem hiding this comment.
P2: Since we no longer set topP anywhere for Bedrock, a brief note here like "Bedrock requires choosing between temperature and topP; we standardize on temperature" would make the policy explicit at both call sites.
|
|
||
| // Should have default temperature (0.3) but not topP | ||
| expect(commandArg.inferenceConfig).toBeDefined() | ||
| expect(commandArg.inferenceConfig.temperature).toBe(0.3) // BEDROCK_DEFAULT_TEMPERATURE |
There was a problem hiding this comment.
P2: The test hardcodes 0.3 as the default temperature. To reduce brittleness if defaults change, consider centralizing the default in a shared constant in source and importing it here (or deriving it from the provider), rather than duplicating the literal.
Description
This PR fixes the error "temperature and top_p cannot both be specified for this model" that occurs when using Claude Sonnet 3.5 v2 (also referred to as Sonnet 4.5) with AWS Bedrock.
Problem
AWS Bedrock's implementation of certain Claude models doesn't allow both
temperatureandtopPparameters to be specified simultaneously. When both are sent, the API returns an error preventing the model from being used.Solution
topPparameter from the inference configuration for all Bedrock modelstemperatureparameter is now sent to AWS BedrockChanges
src/api/providers/bedrock.tsto removetopPfrom inference config in bothcreateMessage()andcompletePrompt()methodssrc/api/providers/__tests__/bedrock-temperature-top-p.spec.tswith 5 test cases covering:Testing
Related Issue
Fixes #8377
Important
Remove
topPparameter from AWS Bedrock inference configuration to fix compatibility issues, ensuring onlytemperatureis used.topPparameter from inference configuration increateMessage()andcompletePrompt()inbedrock.ts.temperatureis sent to AWS Bedrock to avoid compatibility errors.bedrock-temperature-top-p.spec.tswith 5 test cases for different models and configurations.topPis not included andtemperatureis correctly handled.This description was created by
for 9a44fe5. You can customize this summary. It will automatically update as commits are pushed.