-
Notifications
You must be signed in to change notification settings - Fork 747
MAINT: Remove unused fields in MessagePiece #1616
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
base: main
Are you sure you want to change the base?
Changes from all commits
303cafc
940e0e2
98d054c
df19b36
8d8cc16
c333ba3
06e9d71
8e58b47
9508f75
b64c9ab
0d7958f
4940436
cc76142
a203006
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ | |
|
|
||
| import uuid | ||
| from datetime import datetime, timezone | ||
| from typing import TYPE_CHECKING, Any, Literal, Optional, Union, get_args | ||
| from typing import TYPE_CHECKING, Any, Optional, Union, get_args | ||
| from uuid import uuid4 | ||
|
|
||
| from pyrit.identifiers.component_identifier import ComponentIdentifier | ||
|
|
@@ -14,8 +14,6 @@ | |
| if TYPE_CHECKING: | ||
| from pyrit.models.score import Score | ||
|
|
||
| Originator = Literal["attack", "converter", "undefined", "scorer"] | ||
|
|
||
|
|
||
| class MessagePiece: | ||
| """ | ||
|
|
@@ -42,15 +40,11 @@ def __init__( | |
| converter_identifiers: Optional[list[Union[ComponentIdentifier, dict[str, str]]]] = None, | ||
| prompt_target_identifier: Optional[Union[ComponentIdentifier, dict[str, Any]]] = None, | ||
| attack_identifier: Optional[Union[ComponentIdentifier, dict[str, str]]] = None, | ||
| scorer_identifier: Optional[Union[ComponentIdentifier, dict[str, str]]] = None, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we get rid of scorer_identifier but not the other identifiers ?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because scores should include it
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This particular one, along with other fields here, is just never populated in other classes - and this PR is only focusing on such unused fields that are never populated, so there's no point in having them. Other ones could be in scope for removal, for example attack_identifier is also duplicated between message pieces and attack_results ... but the difference is, that is populated in other classes today, and will have a more cascading change all over the repo. |
||
| original_value_data_type: PromptDataType = "text", | ||
| converted_value_data_type: Optional[PromptDataType] = None, | ||
| response_error: PromptResponseError = "none", | ||
| originator: Originator = "undefined", | ||
| original_prompt_id: Optional[uuid.UUID] = None, | ||
| timestamp: Optional[datetime] = None, | ||
| scores: Optional[list[Score]] = None, | ||
| targeted_harm_categories: Optional[list[str]] = None, | ||
| ): | ||
| """ | ||
| Initialize a MessagePiece. | ||
|
|
@@ -74,16 +68,11 @@ def __init__( | |
| objects or dicts (deprecated, will be removed in 0.14.0). Defaults to None. | ||
| prompt_target_identifier: The target identifier for the prompt. Defaults to None. | ||
| attack_identifier: The attack identifier for the prompt. Defaults to None. | ||
| scorer_identifier: The scorer identifier for the prompt. Accepts a ComponentIdentifier. | ||
| Defaults to None. | ||
| original_value_data_type: The data type of the original prompt (text, image). Defaults to "text". | ||
| converted_value_data_type: The data type of the converted prompt (text, image). Defaults to "text". | ||
| response_error: The response error type. Defaults to "none". | ||
| originator: The originator of the prompt. Defaults to "undefined". | ||
| original_prompt_id: The original prompt id. It is equal to id unless it is a duplicate. Defaults to None. | ||
| timestamp: The timestamp of the memory entry. Defaults to None (auto-generated). | ||
| scores: The scores associated with the prompt. Defaults to None. | ||
| targeted_harm_categories: The harm categories associated with the prompt. Defaults to None. | ||
|
|
||
| Raises: | ||
| ValueError: If role, data types, or response error are invalid. | ||
|
|
@@ -134,11 +123,6 @@ def __init__( | |
| ComponentIdentifier.normalize(attack_identifier) if attack_identifier else None | ||
| ) | ||
|
|
||
| # Handle scorer_identifier: normalize to ComponentIdentifier (handles dict with deprecation warning) | ||
| self.scorer_identifier: Optional[ComponentIdentifier] = ( | ||
| ComponentIdentifier.normalize(scorer_identifier) if scorer_identifier else None | ||
| ) | ||
|
|
||
| self.original_value = original_value | ||
|
|
||
| if original_value_data_type not in get_args(PromptDataType): | ||
|
|
@@ -161,13 +145,21 @@ def __init__( | |
| raise ValueError(f"response_error {response_error} is not a valid response error.") | ||
|
|
||
| self.response_error = response_error | ||
| self.originator = originator | ||
|
|
||
| # Original prompt id defaults to id (assumes that this is the original prompt, not a duplicate) | ||
| self.original_prompt_id = original_prompt_id or self.id | ||
|
|
||
| self.scores = scores if scores else [] | ||
| self.targeted_harm_categories = targeted_harm_categories if targeted_harm_categories else [] | ||
| # Scores are not set via constructor. They are hydrated by the memory layer | ||
| # via _set_scores() after construction. | ||
| self._scores: list[Score] = [] | ||
|
|
||
| @property | ||
| def scores(self) -> list[Score]: | ||
| """Scores associated with this message piece, hydrated by the memory layer.""" | ||
| return list(self._scores) | ||
|
|
||
| def _set_scores(self, scores: list[Score]) -> None: | ||
| self._scores = scores | ||
|
behnam-o marked this conversation as resolved.
|
||
|
|
||
| async def set_sha256_values_async(self) -> None: | ||
| """ | ||
|
|
@@ -280,22 +272,19 @@ def to_dict(self) -> dict[str, object]: | |
| "sequence": self.sequence, | ||
| "timestamp": self.timestamp.isoformat() if self.timestamp else None, | ||
| "labels": self.labels, | ||
| "targeted_harm_categories": self.targeted_harm_categories if self.targeted_harm_categories else None, | ||
| "prompt_metadata": self.prompt_metadata, | ||
| "converter_identifiers": [conv.to_dict() for conv in self.converter_identifiers], | ||
| "prompt_target_identifier": ( | ||
| self.prompt_target_identifier.to_dict() if self.prompt_target_identifier else None | ||
| ), | ||
| "attack_identifier": self.attack_identifier.to_dict() if self.attack_identifier else None, | ||
| "scorer_identifier": self.scorer_identifier.to_dict() if self.scorer_identifier else None, | ||
| "original_value_data_type": self.original_value_data_type, | ||
| "original_value": self.original_value, | ||
| "original_value_sha256": self.original_value_sha256, | ||
| "converted_value_data_type": self.converted_value_data_type, | ||
| "converted_value": self.converted_value, | ||
| "converted_value_sha256": self.converted_value_sha256, | ||
| "response_error": self.response_error, | ||
| "originator": self.originator, | ||
| "original_prompt_id": str(self.original_prompt_id), | ||
| "scores": [score.to_dict() for score in self.scores], | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.