feat: add suppressed_params support to BedrockGenerator#1842
Open
u7k4rs6 wants to merge 1 commit into
Open
Conversation
Signed-off-by: u7k4rs6 <utkarshbahuguna10@gmail.com>
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.
Refs #1812
Why added
Issue #1812 surfaced that BedrockGenerator cannot send a Bedrock-compatible inferenceConfig for target models with parameter restrictions: Anthropic Claude 4.x on Bedrock rejects requests that set both temperature and topP. In comments on #1812, @jmartin-tech proposed a model-agnostic mechanism: support suppressed_params on BedrockGenerator the same way OpenAICompatible generators do, so operators can globally suppress any parameter known to be unsupported by their target.
This PR implements that suggestion.
Relationship to PR #1729 (merged)
PR #1729 added configurability at the probe layer (
garak/probes/promptinject.py) for which generation_params promptinject's_generator_precall_hookoverrides. This PR operates at the generator layer (garak/generators/bedrock.py), providing suppression for any Bedrock target regardless of which probe is running. The two mechanisms compose: #1729 addresses the promptinject-specific override behavior, this PR addresses the broader case where a Bedrock model rejects a parameter combination from any source.Relationship to PR #1840 (open)
PR #1840 patches the immediate Anthropic case with a model-name-gated conditional in
_call_model. This PR is the generic mechanism @jmartin-tech suggested in the #1812 thread. Once it lands, the same outcome is achievable via site config (suppressed_params: [topP]) or a per-model default suppression set, without model-name checks in_call_model. The two approaches aren't mutually exclusive, but the generic mechanism subsumes the special case.How utilized
In
garak.site.yaml:Then run garak normally:
The
topPfield is absent from everyinferenceConfigsent tobedrock-runtime.converse, even when downstream hooks (e.g.,promptinject._generator_precall_hook) mutategenerator.top_pmid-run.Tests
test_inference_config_unchanged_with_empty_suppressed_params: refactored code is output-identical to the original for all four fields.test_suppressed_params_blocks_top_p:topPabsent whensuppressed_params={"topP"}andtop_pis non-None.test_suppression_survives_attribute_mutation: suppression holds aftergenerator.top_pis mutated mid-run, simulatingpromptinject._generator_precall_hook.test_warn_on_pythonic_suppressed_key: WARNING logged at init when a Pythonic name (top_p) is used instead of the API name (topP).All 15 tests in
tests/generators/test_bedrock.pypass.