-
Notifications
You must be signed in to change notification settings - Fork 717
FEAT add TargetRequirements #1582
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.
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
Open
hannahwestra25
wants to merge
5
commits into
microsoft:main
Choose a base branch
from
hannahwestra25:hawestra/add_target_requirements
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+187
−0
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
8820154
add target requirements
hannahwestra25 08168d3
fix test issue and aggregate errors
hannahwestra25 f550310
pre-commit
hannahwestra25 99bb839
Merge branch 'main' of https://github.com/microsoft/PyRIT into hawest…
hannahwestra25 4902add
remove test
hannahwestra25 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| # Copyright (c) Microsoft Corporation. | ||
| # Licensed under the MIT license. | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from dataclasses import dataclass, field | ||
| from typing import TYPE_CHECKING | ||
|
|
||
| if TYPE_CHECKING: | ||
| from pyrit.prompt_target.common.target_capabilities import CapabilityName | ||
| from pyrit.prompt_target.common.target_configuration import TargetConfiguration | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class TargetRequirements: | ||
| """ | ||
| Declarative description of what a consumer (attack, converter, scorer) | ||
| requires from a target. | ||
|
|
||
| Consumers define their requirements once and validate them against a | ||
| ``TargetConfiguration`` at construction time. This replaces ad-hoc | ||
| ``isinstance`` checks and scattered capability branching. | ||
| """ | ||
|
|
||
| # The set of capabilities the consumer requires. | ||
| required_capabilities: frozenset[CapabilityName] = field(default_factory=frozenset) | ||
|
|
||
| def validate(self, *, configuration: TargetConfiguration) -> None: | ||
| """ | ||
| Validate that the target configuration can satisfy all requirements. | ||
|
|
||
| Iterates over every required capability and delegates to | ||
| ``TargetConfiguration.ensure_can_handle``, which checks native support | ||
| first and then consults the handling policy. All violations are | ||
| collected and reported in a single ``ValueError``. | ||
|
|
||
| Args: | ||
| configuration (TargetConfiguration): The target configuration to validate against. | ||
|
|
||
| Raises: | ||
| ValueError: If any required capability is missing and the policy | ||
| does not allow adaptation. | ||
| """ | ||
| errors: list[str] = [] | ||
| for capability in sorted(self.required_capabilities, key=lambda c: c.value): | ||
| try: | ||
| configuration.ensure_can_handle(capability=capability) | ||
| except ValueError as exc: | ||
| errors.append(str(exc)) | ||
| if errors: | ||
| raise ValueError( | ||
| f"Target does not satisfy {len(errors)} required capability(ies):\n" | ||
| + "\n".join(f" - {e}" for e in errors) | ||
| ) | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| # Copyright (c) Microsoft Corporation. | ||
| # Licensed under the MIT license. | ||
|
|
||
| import pytest | ||
|
|
||
| from pyrit.prompt_target.common.target_capabilities import ( | ||
| CapabilityHandlingPolicy, | ||
| CapabilityName, | ||
| TargetCapabilities, | ||
| UnsupportedCapabilityBehavior, | ||
| ) | ||
| from pyrit.prompt_target.common.target_configuration import TargetConfiguration | ||
| from pyrit.prompt_target.common.target_requirements import TargetRequirements | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def adapt_all_policy(): | ||
| return CapabilityHandlingPolicy( | ||
| behaviors={ | ||
| CapabilityName.SYSTEM_PROMPT: UnsupportedCapabilityBehavior.ADAPT, | ||
| CapabilityName.MULTI_TURN: UnsupportedCapabilityBehavior.ADAPT, | ||
| CapabilityName.JSON_SCHEMA: UnsupportedCapabilityBehavior.RAISE, | ||
| CapabilityName.JSON_OUTPUT: UnsupportedCapabilityBehavior.RAISE, | ||
| CapabilityName.MULTI_MESSAGE_PIECES: UnsupportedCapabilityBehavior.RAISE, | ||
| CapabilityName.EDITABLE_HISTORY: UnsupportedCapabilityBehavior.RAISE, | ||
| } | ||
| ) | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Construction | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| def test_init_default_has_empty_capabilities(): | ||
| reqs = TargetRequirements() | ||
| assert reqs.required_capabilities == frozenset() | ||
|
|
||
|
|
||
| def test_init_with_capabilities(): | ||
| reqs = TargetRequirements( | ||
| required_capabilities=frozenset({CapabilityName.MULTI_TURN, CapabilityName.SYSTEM_PROMPT}) | ||
| ) | ||
| assert CapabilityName.MULTI_TURN in reqs.required_capabilities | ||
| assert CapabilityName.SYSTEM_PROMPT in reqs.required_capabilities | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # validate — all pass | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| def test_validate_passes_when_target_supports_all_natively(): | ||
| caps = TargetCapabilities(supports_multi_turn=True, supports_system_prompt=True) | ||
| config = TargetConfiguration(capabilities=caps) | ||
| reqs = TargetRequirements( | ||
| required_capabilities=frozenset({CapabilityName.MULTI_TURN, CapabilityName.SYSTEM_PROMPT}) | ||
| ) | ||
| reqs.validate(configuration=config) | ||
|
|
||
|
|
||
| def test_validate_passes_when_policy_is_adapt(adapt_all_policy): | ||
| caps = TargetCapabilities(supports_multi_turn=False, supports_system_prompt=False) | ||
| config = TargetConfiguration(capabilities=caps, policy=adapt_all_policy) | ||
| reqs = TargetRequirements( | ||
| required_capabilities=frozenset({CapabilityName.MULTI_TURN, CapabilityName.SYSTEM_PROMPT}) | ||
| ) | ||
| reqs.validate(configuration=config) | ||
|
|
||
|
|
||
| def test_validate_passes_with_empty_requirements(): | ||
| caps = TargetCapabilities(supports_multi_turn=True, supports_system_prompt=True) | ||
| config = TargetConfiguration(capabilities=caps) | ||
| reqs = TargetRequirements() | ||
| reqs.validate(configuration=config) | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # validate — failures | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| def test_validate_raises_when_capability_missing_and_no_policy(): | ||
| # EDITABLE_HISTORY has no normalizer and no handling policy — validate raises. | ||
| caps = TargetCapabilities(supports_editable_history=False, supports_multi_turn=True, supports_system_prompt=True) | ||
| config = TargetConfiguration(capabilities=caps) | ||
| reqs = TargetRequirements(required_capabilities=frozenset({CapabilityName.EDITABLE_HISTORY})) | ||
| with pytest.raises(ValueError, match="supports_editable_history"): | ||
| reqs.validate(configuration=config) | ||
|
|
||
|
|
||
| def test_validate_raises_when_capability_missing_and_policy_raise(adapt_all_policy): | ||
| # json_output is missing and the policy is RAISE — validate raises. | ||
| caps = TargetCapabilities(supports_multi_turn=False, supports_system_prompt=False, supports_json_output=False) | ||
| config = TargetConfiguration(capabilities=caps, policy=adapt_all_policy) | ||
| reqs = TargetRequirements(required_capabilities=frozenset({CapabilityName.JSON_OUTPUT})) | ||
| with pytest.raises(ValueError, match="supports_json_output"): | ||
| reqs.validate(configuration=config) | ||
|
|
||
|
|
||
| def test_validate_collects_all_unsatisfied_capabilities(adapt_all_policy): | ||
| """When multiple capabilities are missing, validate reports all violations.""" | ||
| caps = TargetCapabilities( | ||
| supports_multi_turn=False, | ||
| supports_system_prompt=False, | ||
| supports_json_output=False, | ||
| supports_editable_history=False, | ||
| ) | ||
| config = TargetConfiguration(capabilities=caps, policy=adapt_all_policy) | ||
| # json_output => RAISE, editable_history => no policy (raises) | ||
| reqs = TargetRequirements( | ||
| required_capabilities=frozenset({CapabilityName.JSON_OUTPUT, CapabilityName.EDITABLE_HISTORY}) | ||
| ) | ||
| with pytest.raises(ValueError, match="2 required capability") as exc_info: | ||
| reqs.validate(configuration=config) | ||
| assert "supports_json_output" in str(exc_info.value) | ||
| assert "supports_editable_history" in str(exc_info.value) | ||
|
|
||
|
|
||
| def test_validate_mixed_adapt_and_raise(adapt_all_policy): | ||
| """One capability adapts but another raises — validate should raise.""" | ||
| caps = TargetCapabilities(supports_multi_turn=False, supports_system_prompt=False, supports_json_output=False) | ||
| config = TargetConfiguration(capabilities=caps, policy=adapt_all_policy) | ||
| # multi_turn and system_prompt => ADAPT (OK), json_output => RAISE (fail) | ||
| reqs = TargetRequirements( | ||
| required_capabilities=frozenset( | ||
| {CapabilityName.MULTI_TURN, CapabilityName.SYSTEM_PROMPT, CapabilityName.JSON_OUTPUT} | ||
| ) | ||
| ) | ||
| with pytest.raises(ValueError, match="supports_json_output"): | ||
| reqs.validate(configuration=config) |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I'm not sure I understand the point of this dataclass. What does this do that a validate method attached to
TargetConfigurationorTargetCapabilitiesdoesn't? AttachingTargetCapabilitiesto a consumer object is weird but this class seems like it could be reduced to a private attribute for consumers (_REQUIRED_CAPABILITIES: frozenset) while the validation method is moved elsewhere. Non-blocking comment since this works but worth documentingThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah so the main motivation for the class itself is to have a descriptive interface so users (converters, scorers, attacks) can have a REQUIRED_CAPABILITIES member variable of the type TargetRequirements. I prefer having the two classes bc they are two separate concepts--what a target can support vs what a user of the target requires so I think it's more straightforward. The other place I could see a validation function (that would reduce duplication of the validation func) is in the TargetConfiguration class but it would be a TargetConfiguration instance verifying that another TargetConfiguration can support it vs a TargetConfiguration instance verifying that it meets TargetRequirements. wdyt ? is there a more natural place to put the validation function?