Skip to content

MAINT: Refactor Skeleton Key to be a subclass of PromptSendingOrchest…#650

Closed
Maalvi14 wants to merge 1 commit into
microsoft:mainfrom
Maalvi14:MAINT--Refactor-Skeleton-Key-to-be-a-subclass-of-PromptSendingOrchestrator-#599
Closed

MAINT: Refactor Skeleton Key to be a subclass of PromptSendingOrchest…#650
Maalvi14 wants to merge 1 commit into
microsoft:mainfrom
Maalvi14:MAINT--Refactor-Skeleton-Key-to-be-a-subclass-of-PromptSendingOrchestrator-#599

Conversation

@Maalvi14
Copy link
Copy Markdown

Overview

This PR refactors the SkeletonKeyOrchestrator to properly inherit from PromptSendingOrchestrator instead of Orchestrator. The refactoring improves code organization and reliability while maintaining all existing functionality. It focuses on leveraging the parent class's capabilities for prompt handling and conversation management.

Key Features

Method Refactoring:

  • Rewrote send_skeleton_key_with_prompts_async to use parent class's batching capabilities
  • Enhanced send_skeleton_key_with_prompt_async to maintain consistent conversation context
  • Removed redundant prompt sending logic by leveraging parent class methods
  • Introduced proper use of _create_normalizer_request for request creation

Request Handling:

  • Added single conversation ID management across all prompts in a sequence
  • Implemented proper metadata propagation through request chain
  • Enhanced RPM validation to use correct attribute checks
  • Improved batch processing of skeleton key and attack prompts

Memory Management:

  • Leveraged parent class's memory tracking functionality
  • Fixed duplicate memory entries in conversation history
  • Ensured proper conversation linking between skeleton key and attack prompts
  • Maintained correct memory entry count for all scenarios

Backward Compatibility:

  • Maintained existing public API interface
  • Preserved custom skeleton key functionality
  • Retained support for prompt converters
  • Kept existing RPM validation capabilities

Related Issues

MAINT: Refactor Skeleton Key to be a subclass of PromptSendingOrchestrator #599

Next Steps (DONE):

  • ✅ Refactored core methods to use PromptSendingOrchestrator functionality
  • ✅ Fixed conversation ID handling in both single and multi-prompt scenarios
  • ✅ Corrected memory entry management
  • ✅ Updated RPM validation logic
  • ✅ Verified all 13 unit tests pass successfully
  • ✅ Confirmed backward compatibility with existing features

@Maalvi14
Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

PromptConverterConfiguration,
)
from pyrit.prompt_target import PromptTarget
from pyrit.orchestrator.single_turn.prompt_sending_orchestrator import PromptSendingOrchestrator
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from pyrit.orchestrator.single_turn.prompt_sending_orchestrator import PromptSendingOrchestrator
from pyrit.orchestrator.single_turn import PromptSendingOrchestrator



class SkeletonKeyOrchestrator(Orchestrator):
class SkeletonKeyOrchestrator(PromptSendingOrchestrator):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommend moving this file to pyrit/orchestrator/single_turn

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(and as part of that move, updating init.py

PromptRequestResponse: The response from the prompt target.
list[PromptRequestResponse]: The responses from the prompt target.
"""
if hasattr(self._prompt_target, 'rpm') and self._prompt_target.rpm and self._batch_size != 1:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be removed - it's the responsibility of the target to check this


# Create a single conversation ID for the entire sequence
conversation_id = str(uuid4())
metadata = {"conversation_id": conversation_id}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the conversation_id as metadata. PromptRequestPieces have this as an attribute and it can be set there

# Return the attack prompt response (second response)
return responses[1]

def print_conversation(self) -> None:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should be able to remove print_conversation and use the parent class

Returns:
list[PromptRequestResponse]: The responses from the prompt target.
PromptRequestResponse: The response from the prompt target.
"""
Copy link
Copy Markdown
Contributor

@rlundeen2 rlundeen2 Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of doing it like how it was done, I would make use of PromptSendingOrchestrator.

  1. Use the set_prepended_conversation to set the skeleton key. This can be done in init
  2. You can remove the send_skeleton_key_with_prompt_async function. Sending any prompt will then have the skeleton key prepended

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if you prepend you have to prepend the response as well, right? What would you set as the response?

batch_size=batch_size,
verbose=verbose
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pre-commit is failing. You can run it using pre-commit run --all-files; And you can see the current issues in the CI build: https://github.com/Azure/PyRIT/actions/runs/12876675701/job/35900136914?pr=650

@rlundeen2
Copy link
Copy Markdown
Contributor

Closing for now, but feel free to re-open if you want us to take another look!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MAINT: Refactor Skeleton Key to be a subclass of PromptSendingOrchestrator

3 participants