MAINT Breaking: Creating Conversation class and moving fields off of messagePiece#1950
Open
rlundeen2 wants to merge 4 commits into
Open
MAINT Breaking: Creating Conversation class and moving fields off of messagePiece#1950rlundeen2 wants to merge 4 commits into
rlundeen2 wants to merge 4 commits into
Conversation
Move prompt_target_identifier to a new Conversations table (hydrated on read) and remove attack_identifier from MessagePiece (it now lives only on AttackResult.atomic_attack_identifier). The deprecated attack_id query filter resolves via get_attack_results() to the attack's main conversation. Adds the Conversation model and an alembic migration for the Conversations table, and updates all call sites and unit tests accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Migrate remaining test sites off the removed MessagePiece.prompt_target_identifier field, thread target_identifier through duplication call sites, and update the memory schema/data-type docs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…n-model # Conflicts: # pyrit/memory/azure_sql_memory.py # pyrit/memory/sqlite_memory.py # tests/unit/executor/attack/single_turn/test_skeleton_key.py
Correct the stale ConversationEntry docstring (target metadata is read via get_conversation_metadata, not rehydrated onto pieces) and add a regression test asserting a None target_identifier on a later write does not overwrite the target already recorded for a conversation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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
1 participant
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.
Description
MessagePiece is gigantic - due to deprecating fields via a rename and not having good models. We've moved some pieces to
AttackResult, but one field (TargetIdentifier) was tricky to move off because it is tied to a conversation and not necessarily anAttackResult(e.g. an adversarial conversation can have a differentTargetIdentifier).Phase 12 of pyrit.models refactor here: https://gist.github.com/rlundeen2/3e8daa8e12a11b4b6e52587b3c9b1dca
Why remove deprecated fields?
This PR creates a conversation model, and moves these properties to the correct place. I opted to not deprecate for a couple reasons:
MessagePiecewould have to lazily query theConversationstable per piece, hiding N+1 reads and returning subtly stale/empty values for in-memory pieces that were never persisted. Silent-but-wrong is worse than a loud break.