FEAT: Enforce keyword-only __init__ on PyRIT pluggable brick base classes

.github/copilot-instructions.md

@@ -4,7 +4,7 @@ PyRIT (Python Risk Identification Tool for generative AI) is an open-source fram
 
 ## Architecture
 
-PyRIT uses a modular "Lego brick" design. The main extensibility points are:
+PyRIT uses a modular pluggable-brick design. The main extensibility points are:
 
 - **Prompt Converters** (`pyrit/prompt_converter/`) — Transform prompts (70+ implementations). Base: `PromptConverter`.
 - **Scorers** (`pyrit/score/`) — Evaluate responses. Base: `Scorer`.

.github/instructions/attacks.instructions.md

@@ -0,0 +1,58 @@
+---
+applyTo: "pyrit/executor/attack/**"
+---
+
+# PyRIT AttackStrategy Development Guidelines
+
+`AttackStrategy` subclasses (single-turn attacks like `PromptSendingAttack`, multi-turn attacks like `RedTeamingAttack`, etc.) are pluggable bricks orchestrated by `AttackExecutor` and the `Scenario` framework. Style rules from `style-guide.instructions.md` (async `_async` suffix, keyword-only args, type hints, enums-over-Literals) still apply and are not repeated here.
+
+## Constructor contract
+
+`AttackStrategy` subclasses MUST follow the keyword-only constructor shape:
+
+```python
+class MyAttack(AttackStrategy[MyContext, MyResult]):
+    def __init__(
+        self,
+        *,
+        objective_target: PromptTarget,
+        custom_param: str = "",
+        **kwargs: Any,
+    ) -> None:
+        super().__init__(
+            objective_target=objective_target,
+            context_type=MyContext,
+            **kwargs,
+        )
+```
+
+Requirements:
+
+- All parameters after ``self`` are keyword-only (insert ``*`` immediately
+  after ``self``). This is **enforced at class-definition time** by
+  `AttackStrategy.__init_subclass__` calling `enforce_keyword_only_init`
+  (see `pyrit/common/brick_contract.py`). Non-conforming subclasses
+  raise `TypeError` at import time.
+- ``super().__init__(...)`` must be invoked with at minimum
+  ``objective_target`` and ``context_type``.
+- Existing subclasses that cannot adopt the contract immediately may set
+  the class attribute ``_brick_legacy_init = True`` to opt into a
+  one-release grace period that downgrades the error to a
+  ``DeprecationWarning(removed_in="0.16.0")``. The opt-out is removed in
+  0.16.0; classes that still violate the contract at that point will hard
+  fail.
+- ``AttackTechniqueFactory`` already rejects ``**kwargs`` in attack
+  ``__init__`` at factory-registration time
+  (`pyrit/scenario/core/attack_technique_factory.py`); the new
+  ``__init_subclass__`` check is complementary — the factory check catches
+  scenarios-side wiring mistakes, the subclass check catches the
+  ``__init__`` shape at class-definition time.
+
+## Common pitfalls
+
+- Forgetting ``*`` after ``self`` — the new check will surface this at
+  import time with the exact list of positional parameters that need to be
+  made keyword-only.
+- Calling ``super().__init__`` with positional arguments — the base
+  ``AttackStrategy.__init__`` is already keyword-only, so positional calls
+  raise ``TypeError`` at runtime. Always forward via kwargs.

.github/instructions/converters.instructions.md

@@ -80,6 +80,42 @@ class MyConverter(PromptConverter):
         ...
 ```
 
+### Keyword-only ``__init__`` is enforced
+
+Every ``PromptConverter`` subclass MUST make all ``__init__`` parameters
+keyword-only (i.e., place ``*`` as the first parameter after ``self``).
+``PromptConverter.__init_subclass__`` validates this at class-definition
+time via ``enforce_keyword_only_init`` and raises ``TypeError`` on
+violations.
+
+The check is satisfied by either of:
+
+```python
+def __init__(self, *, foo: str, bar: int = 0) -> None: ...
+
+def __init__(self, *args: str, foo: str = "") -> None: ...  # *args after self
+```
+
+It rejects:
+
+```python
+def __init__(self, foo: str, bar: int = 0) -> None: ...    # missing *
+```
+
+### Temporary opt-out: ``_brick_legacy_init``
+
+A handful of legacy converters whose positional ``__init__`` is part of the
+public API are grandfathered with ``_brick_legacy_init = True``. They
+emit a ``DeprecationWarning`` at import time and the opt-out is scheduled
+for removal in **0.16.0**. Do not set this flag on new converters; new
+converters MUST follow the keyword-only contract.
+
+Currently grandfathered (slated for cleanup in 0.16.0):
+``AddImageVideoConverter``, ``AnsiAttackConverter``, ``AsciiArtConverter``,
+``AskToDecodeConverter``, ``DiacriticConverter``, ``InsertPunctuationConverter``,
+``PDFConverter``, ``QRCodeConverter``, ``RandomCapitalLettersConverter``,
+``SearchReplaceConverter``, ``SmugglerConverter`` (and its three subclasses).
+
 ## Exports and External Updates
 
 - New converters MUST be added to `pyrit/prompt_converter/__init__.py` — both the import and the `__all__` list.

.github/instructions/datasets.instructions.md

@@ -7,6 +7,13 @@ applyTo: "pyrit/datasets/seed_datasets/**"
 These rules apply when adding or modifying loaders under `pyrit/datasets/seed_datasets/`.
 Style rules from `style-guide.instructions.md` (async `_async` suffix, keyword-only args, type hints, enums-over-Literals) still apply and are not repeated here.
 
+The keyword-only `__init__` rule is **enforced at class-definition time** by
+`SeedDatasetProvider.__init_subclass__` calling `enforce_keyword_only_init` (see
+`pyrit/common/brick_contract.py`). Loaders with positional `__init__` params raise
+`TypeError` at import time; existing offenders may set `_brick_legacy_init = True`
+to opt into a one-release grace period that downgrades the error to a
+`DeprecationWarning(removed_in="0.16.0")`.
+
 ## Use SeedObjective for behavior/goal rows; SeedPrompt for literal messages
 
 This is the most consequential modelling decision and must be made before writing the loader:

.github/instructions/scenarios.instructions.md

@@ -71,7 +71,13 @@ def __init__(
 
 Requirements:
 - `@apply_defaults` decorator on `__init__`
-- All parameters keyword-only via `*`
+- All parameters keyword-only via `*` — **enforced at class-definition time** by
+  `Scenario.__init_subclass__` calling `enforce_keyword_only_init` (see
+  `pyrit/common/brick_contract.py`). Violators raise `TypeError` at
+  import time. Existing classes that cannot adopt the contract immediately
+  may opt into a one-release grace period via the class attribute
+  `_brick_legacy_init = True`, which downgrades the error to a
+  `DeprecationWarning(removed_in="0.16.0")`. The opt-out is removed in 0.16.0.
 - **All constructor parameters must be optional** (default to `None`) so the registry can instantiate the scenario with no arguments for metadata introspection. Defer required-input validation to `initialize_async()` or `_get_atomic_attacks_async()`. `ScenarioRegistry._build_metadata` raises `TypeError` if `scenario_class()` cannot be called with no arguments.
 - `super().__init__()` called with `version`, `strategy_class`, `default_strategy`, `default_dataset_config`, `objective_scorer`
 - complex objects like `adversarial_chat` or `objective_scorer` should be passed into the constructor.

.github/instructions/scorers.instructions.md

@@ -0,0 +1,60 @@
+---
+applyTo: "pyrit/score/**"
+---
+
+# PyRIT Scorer Development Guidelines
+
+Scorers evaluate model responses against an objective and live under `pyrit/score/`. Style rules from `style-guide.instructions.md` (async `_async` suffix, keyword-only args, type hints, enums-over-Literals) still apply and are not repeated here.
+
+## Constructor contract
+
+`Scorer` subclasses MUST use the keyword-only constructor shape:
+
+```python
+class MyScorer(Scorer):
+    def __init__(
+        self,
+        *,
+        chat_target: PromptTarget | None = None,
+        threshold: float = 0.5,
+        validator: ScorerPromptValidator | None = None,
+    ) -> None:
+        super().__init__(
+            validator=validator or self._DEFAULT_VALIDATOR,
+            chat_target=chat_target,
+        )
+```
+
+Requirements:
+
+- All parameters after ``self`` are keyword-only (insert ``*`` immediately
+  after ``self``). This is **enforced at class-definition time** by
+  `Scorer.__init_subclass__` calling `enforce_keyword_only_init`
+  (see `pyrit/common/brick_contract.py`). Non-conforming subclasses
+  raise `TypeError` at import time.
+- ``super().__init__(validator=..., chat_target=...)`` is required so the
+  base class wires the validator and validates ``TARGET_REQUIREMENTS``
+  against any provided ``chat_target``.
+- Existing subclasses that cannot adopt the contract immediately may set
+  the class attribute ``_brick_legacy_init = True`` to opt into a
+  one-release grace period that downgrades the error to a
+  ``DeprecationWarning(removed_in="0.16.0")``. The opt-out is removed in
+  0.16.0; classes that still violate the contract at that point will hard
+  fail.
+
+### Currently grandfathered
+
+- ``PlagiarismScorer`` (``pyrit/score/float_scale/plagiarism_scorer.py``) —
+  accepts ``reference_text`` positionally as part of its public API. The
+  positional shape is preserved through one release cycle via
+  ``_brick_legacy_init = True`` and is scheduled to become
+  keyword-only in 0.16.0 (``BREAKING CHANGE``).
+
+## Common pitfalls
+
+- Forgetting ``*`` after ``self`` — the new check will surface this at
+  import time with the exact list of positional parameters that need to be
+  made keyword-only.
+- Calling ``super().__init__`` with positional args — the base
+  ``Scorer.__init__`` is already keyword-only, so positional calls raise
+  ``TypeError`` at runtime. Always forward via kwargs.

.github/instructions/targets.instructions.md

@@ -0,0 +1,111 @@
+---
+applyTo: "pyrit/prompt_target/**"
+---
+
+# Prompt Target Development Guidelines
+
+## Base Class Contract
+
+All targets MUST inherit from ``PromptTarget`` (or one of its public
+subclasses such as ``OpenAITarget`` / ``HTTPTarget``) and implement
+``_send_prompt_to_target_async``:
+
+```python
+from pyrit.prompt_target.common.prompt_target import PromptTarget
+
+
+class MyTarget(PromptTarget):
+    def __init__(
+        self,
+        *,
+        endpoint: str,
+        api_key: str,
+        max_requests_per_minute: int | None = None,
+        custom_configuration: TargetConfiguration | None = None,
+    ) -> None:
+        super().__init__(
+            endpoint=endpoint,
+            max_requests_per_minute=max_requests_per_minute,
+            custom_configuration=custom_configuration,
+        )
+        self._api_key = api_key
+
+    async def _send_prompt_to_target_async(
+        self, *, normalized_conversation: list[Message]
+    ) -> list[Message]:
+        ...
+```
+
+``send_prompt_async`` (the public entry point) is ``@final`` and MUST NOT
+be overridden. Override ``_send_prompt_to_target_async`` instead.
+
+## Keyword-only ``__init__`` is enforced
+
+Every ``PromptTarget`` subclass MUST make all ``__init__`` parameters
+keyword-only (i.e., place ``*`` as the first parameter after ``self``).
+``PromptTarget.__init_subclass__`` validates this at class-definition time
+via ``enforce_keyword_only_init`` and raises ``TypeError`` on violations.
+
+The check is satisfied by either of:
+
+```python
+def __init__(self, *, endpoint: str, api_key: str) -> None: ...
+
+def __init__(self, *args: Any, **kwargs: Any) -> None: ...  # *args after self
+```
+
+It rejects:
+
+```python
+def __init__(self, endpoint: str, api_key: str) -> None: ...    # missing *
+```
+
+> [!NOTE]
+> ``PromptTarget.__init__`` *itself* still accepts positional parameters and
+> is not currently keyword-only. The ``__init_subclass__`` hook only runs for
+> subclasses, so the base class non-compliance is tolerated during the warn-
+> first phase. The base ``__init__`` will be reshaped to be keyword-only in
+> 0.16.0 as a BREAKING CHANGE.
+
+## Temporary opt-out: ``_brick_legacy_init``
+
+A handful of legacy targets whose positional ``__init__`` is part of the
+public API are grandfathered with ``_brick_legacy_init = True``. They
+emit a ``DeprecationWarning`` at import time and the opt-out is scheduled
+for removal in **0.16.0**. Do not set this flag on new targets; new
+targets MUST follow the keyword-only contract.
+
+Currently grandfathered (slated for cleanup in 0.16.0):
+``HTTPTarget``, ``OpenAICompletionTarget``, ``OpenAIImageTarget``,
+``PromptShieldTarget``.
+
+## Configuration and Capabilities
+
+- Set ``_DEFAULT_CONFIGURATION`` at the class level when your target's
+  capabilities differ from the base defaults (multi-turn support, non-text
+  modalities, JSON-mode responses, etc.).
+- Accept ``custom_configuration: TargetConfiguration | None = None`` in
+  ``__init__`` and forward it to ``super().__init__`` so callers can
+  override capabilities per-instance (this is required for HTTP / Playwright
+  targets whose capabilities depend on deployment configuration).
+
+## Identifiable Pattern
+
+All targets inherit ``Identifiable``. Override ``_build_identifier()`` to
+include parameters that affect target behaviour:
+
+```python
+def _build_identifier(self) -> ComponentIdentifier:
+    return self._create_identifier(
+        params={"endpoint": self._endpoint, "model_name": self._model_name},
+    )
+```
+
+Include: endpoint, model_name, deployment identifiers, custom headers that
+affect routing.
+Exclude: API keys, retry counts, logging config, timeouts.
+
+## Exports
+
+New targets MUST be added to ``pyrit/prompt_target/__init__.py`` — both
+the import and the ``__all__`` list.

doc/code/framework.md

@@ -67,7 +67,7 @@ The main components of PyRIT are prompts, attacks, converters, targets, and scor
 
 ![alt text](../../assets/architecture_components.png)
 
-As much as possible, each component is a Lego-brick of functionality. Prompts from one attack can be used in another. An attack for one scenario can use multiple targets. And sometimes you completely skip components (e.g. almost every component can be a NoOp also, you can have a NoOp converter that doesn't convert, or a NoOp target that just prints the prompts).
+As much as possible, each component is a pluggable brick of functionality. Prompts from one attack can be used in another. An attack for one scenario can use multiple targets. And sometimes you completely skip components (e.g. almost every component can be a NoOp also, you can have a NoOp converter that doesn't convert, or a NoOp target that just prints the prompts).
 
 If you are contributing to PyRIT, that work will most likely land in one of these buckets and be as self-contained as possible. It isn't always this clean, but when an attack scenario doesn't quite fit (and that's okay!) it's good to brainstorm with the maintainers about how we can modify our architecture.
 

pyrit/common/__init__.py

@@ -20,6 +20,7 @@
     reset_default_values,
     set_default_value,
 )
+from pyrit.common.brick_contract import enforce_keyword_only_init
 from pyrit.common.default_values import get_non_required_value, get_required_value
 from pyrit.common.deprecation import print_deprecation_message
 from pyrit.common.notebook_utils import is_in_ipython_session
@@ -41,6 +42,7 @@
     "combine_dict",
     "combine_list",
     "DefaultValueScope",
+    "enforce_keyword_only_init",
     "get_global_default_values",
     "get_kwarg_param",
     "get_non_required_value",